Michel2/pybricks-code
1
0
Fork 0
mirror of https://github.com/pybricks/pybricks-code.git synced 2026-09-12 01:23:52 +00:00
Code Issues Releases Activity

settings: convert hubName to react hook

Browse Source
This commit is contained in:
David Lechner
2022-03-15 17:25:20 -05:00
parent 861ea34d86
commit 7726dbe01d
14 changed files with 125 additions and 358 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/firmware/FlashButton.tsx
+8 -3
View File
@@ -7,7 +7,7 @@ import { BleConnectionState } from '../ble/reducers';
import { BootloaderConnectionState } from '../lwp3-bootloader/reducers';
import * as notificationActions from '../notifications/actions';
import { useSelector } from '../reducers';
import { useSettingFlashCurrentProgram } from '../settings/hooks';
import { useSettingFlashCurrentProgram, useSettingHubName } from '../settings/hooks';
import OpenFileButton, { OpenFileButtonProps } from '../toolbar/OpenFileButton';
import { TooltipId } from '../toolbar/i18n';
import { flashFirmware } from './actions';
@@ -21,6 +21,7 @@ const FlashButton: React.VoidFunctionComponent<FlashButtonProps> = ({ id }) => {
const flashing = useSelector((s) => s.firmware.flashing);
const progress = useSelector((s) => s.firmware.progress);
const [isSettingFlashCurrentProgramEnabled] = useSettingFlashCurrentProgram();
const { hubName } = useSettingHubName();
const dispatch = useDispatch();
@@ -37,7 +38,9 @@ const FlashButton: React.VoidFunctionComponent<FlashButtonProps> = ({ id }) => {
showProgress={flashing}
progress={progress === null ? undefined : progress}
onFile={(data) =>
dispatch(flashFirmware(data, isSettingFlashCurrentProgramEnabled))
dispatch(
flashFirmware(data, isSettingFlashCurrentProgramEnabled, hubName),
)
}
onReject={(file) =>
dispatch(
@@ -48,7 +51,9 @@ const FlashButton: React.VoidFunctionComponent<FlashButtonProps> = ({ id }) => {
)
}
onClick={() =>
dispatch(flashFirmware(null, isSettingFlashCurrentProgramEnabled))
dispatch(
flashFirmware(null, isSettingFlashCurrentProgramEnabled, hubName),
)
}
/>
);
src/firmware/actions.ts
+3 -1
View File
@@ -122,12 +122,14 @@ export type FailToFinishReason =
* @param data The firmware zip file data or `null` to get firmware later.
* @param flashCurrentProgram If true, flash the current program from the editor,
* otherwise use the program from firmware.zip.
* @param hubName A custom hub name or an empty string to use the default name.
*/
export const flashFirmware = createAction(
(data: ArrayBuffer | null, flashCurrentProgram: boolean) => ({
(data: ArrayBuffer | null, flashCurrentProgram: boolean, hubName: string) => ({
type: 'flashFirmware.action.flashFirmware',
data,
flashCurrentProgram,
hubName,
}),
);
src/firmware/sagas.test.ts
+21 -16
View File
@@ -82,11 +82,9 @@ describe('flashFirmware', () => {
nextMessageId: createCountFunc(),
});
saga.updateState({ settings: { hubName: 'test name' } });
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, 'test name'));
// first step is to connect to the hub bootloader
@@ -231,7 +229,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -280,7 +278,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -347,7 +345,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -410,7 +408,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -476,7 +474,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -535,7 +533,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -600,7 +598,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -681,7 +679,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -746,7 +744,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -840,7 +838,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -943,7 +941,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -1086,7 +1084,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
@@ -1231,6 +1229,7 @@ describe('flashFirmware', () => {
flashFirmwareAction(
await zip.generateAsync({ type: 'arraybuffer' }),
false,
'',
),
);
@@ -1378,6 +1377,7 @@ describe('flashFirmware', () => {
flashFirmwareAction(
await zip.generateAsync({ type: 'arraybuffer' }),
false,
'',
),
);
@@ -1424,6 +1424,7 @@ describe('flashFirmware', () => {
flashFirmwareAction(
await zip.generateAsync({ type: 'arraybuffer' }),
false,
'',
),
);
@@ -1469,6 +1470,7 @@ describe('flashFirmware', () => {
flashFirmwareAction(
await zip.generateAsync({ type: 'arraybuffer' }),
false,
'',
),
);
@@ -1527,6 +1529,7 @@ describe('flashFirmware', () => {
flashFirmwareAction(
await zip.generateAsync({ type: 'arraybuffer' }),
false,
'',
),
);
@@ -1586,6 +1589,7 @@ describe('flashFirmware', () => {
flashFirmwareAction(
await zip.generateAsync({ type: 'arraybuffer' }),
false,
'',
),
);
@@ -1648,6 +1652,7 @@ describe('flashFirmware', () => {
flashFirmwareAction(
await zip.generateAsync({ type: 'arraybuffer' }),
false,
'',
),
);
@@ -1736,7 +1741,7 @@ describe('flashFirmware', () => {
// saga is triggered by this action
saga.put(flashFirmwareAction(null, false));
saga.put(flashFirmwareAction(null, false, ''));
// first step is to connect to the hub bootloader
src/firmware/sagas.ts
+11 -4
View File
@@ -182,6 +182,7 @@ function* firmwareIterator(data: DataView, maxSize: number): Generator<number> {
function* loadFirmware(
data: ArrayBuffer,
program: string | undefined,
hubName: string,
): SagaGenerator<{ firmware: Uint8Array; deviceId: HubType }> {
const [reader, readerErr] = yield* call(() => maybe(FirmwareReader.load(data)));
@@ -247,8 +248,6 @@ function* loadFirmware(
// if the firmware supports it, we can set a custom hub name
if (metadata['max-hub-name-size']) {
const hubName = yield* select((s: RootState) => s.settings.hubName);
// empty string means use default name (don't write over firmware)
if (hubName) {
firmware.set(encodeHubName(hubName, metadata), metadata['hub-name-offset']);
@@ -299,7 +298,11 @@ function* handleFlashFirmware(action: ReturnType<typeof flashFirmware>): Generat
}
if (action.data !== null) {
({ firmware, deviceId } = yield* loadFirmware(action.data, program));
({ firmware, deviceId } = yield* loadFirmware(
action.data,
program,
action.hubName,
));
}
yield* put(connect());
@@ -341,7 +344,11 @@ function* handleFlashFirmware(action: ReturnType<typeof flashFirmware>): Generat
}
const data = yield* call(() => response.arrayBuffer());
({ firmware, deviceId } = yield* loadFirmware(data, program));
({ firmware, deviceId } = yield* loadFirmware(
data,
program,
action.hubName,
));
if (deviceId !== undefined && info.hubType !== deviceId) {
yield* put(didFailToFinish(FailToFinishReasonType.DeviceMismatch));
src/reducers.ts
-2
View File
@@ -10,7 +10,6 @@ import firmware from './firmware/reducers';
import hub from './hub/reducers';
import licenses from './licenses/reducers';
import bootloader from './lwp3-bootloader/reducers';
import settings from './settings/reducers';
/**
* Root reducer for redux store.
@@ -23,7 +22,6 @@ export const rootReducer = combineReducers({
firmware,
hub,
licenses,
settings,
});
/**
src/settings/SettingsDrawer.test.tsx
+28
View File
@@ -55,6 +55,34 @@ describe('flashCurrentProgram setting switch', () => {
});
});
describe('hubName setting', () => {
it('should migrate old settings', () => {
// old settings did not use json format, so lack quotes
localStorage.setItem('setting.hubName', 'old name');
const [settings] = testRender(
<SettingsDrawer isOpen={true} onClose={() => undefined} />,
);
const textBox = settings.getByLabelText('Hub name');
expect(textBox).toHaveValue('old name');
});
it('should update the setting', () => {
const [settings] = testRender(
<SettingsDrawer isOpen={true} onClose={() => undefined} />,
);
expect(localStorage.getItem('setting.hubName')).toBe(null);
const textBox = settings.getByLabelText('Hub name');
userEvent.type(textBox, 'test name');
expect(localStorage.getItem('setting.hubName')).toBe('"test name"');
});
});
describe('about dialog', () => {
it('should open the dialog when the button is clicked', async () => {
const [settings] = testRender(
src/settings/SettingsDrawer.tsx
+7 -13
View File
@@ -35,9 +35,11 @@ import { pseudolocalize } from '../i18n';
import { useSelector } from '../reducers';
import ExternalLinkIcon from '../utils/ExternalLinkIcon';
import { isMacOS } from '../utils/os';
import { setString } from './actions';
import { StringSettingId } from './defaults';
import { useSettingFlashCurrentProgram, useSettingIsShowDocsEnabled } from './hooks';
import {
useSettingFlashCurrentProgram,
useSettingHubName,
useSettingIsShowDocsEnabled,
} from './hooks';
import { SettingsStringId } from './i18n';
import en from './i18n.en.json';
import './settings.scss';
@@ -71,8 +73,7 @@ const SettingsDrawer: React.VoidFunctionComponent<SettingsProps> = ({
);
const promptingInstall = useSelector((s) => s.app.promptingInstall);
const readyForOfflineUse = useSelector((s) => s.app.readyForOfflineUse);
const hubName = useSelector((s) => s.settings.hubName);
const isHubNameValid = useSelector((s) => s.settings.isHubNameValid);
const { hubName, isHubNameValid, setHubName } = useSettingHubName();
const dispatch = useDispatch();
@@ -200,14 +201,7 @@ const SettingsDrawer: React.VoidFunctionComponent<SettingsProps> = ({
<InputGroup
id="hub-name-input"
value={hubName}
onChange={(e) =>
dispatch(
setString(
StringSettingId.HubName,
e.currentTarget.value,
),
)
}
onChange={(e) => setHubName(e.currentTarget.value)}
onMouseOver={(e) => e.preventDefault()}
className="pb-hub-name-input"
intent={isHubNameValid ? Intent.NONE : Intent.DANGER}
src/settings/actions.ts
+1 -24
View File
@@ -1,30 +1,7 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2021-2022 The Pybricks Authors
// Copyright (c) 2022 The Pybricks Authors
import { createAction } from '../actions';
import { StringSettingId } from './defaults';
/** Creates an action to set/store a setting. */
export const setString = createAction((id: StringSettingId, newState: string) => ({
type: 'settings.action.setString',
id,
newState,
}));
/** Creates an action indicating that setting/storing a setting failed. */
export const didFailToSetString = createAction((id: StringSettingId, err: Error) => ({
type: 'settings.action.didFailToSetString',
id,
err,
}));
export const didStringChange = createAction(
(id: StringSettingId, newState: string) => ({
type: 'settings.action.didStringChange',
id,
newState,
}),
);
/** Requests to toggle the showDocs setting. */
export const settingsToggleShowDocs = createAction(() => ({
src/settings/defaults.ts
-18
View File
@@ -1,18 +0,0 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2021-2022 The Pybricks Authors
// Definitions for user selectable settings.
export enum StringSettingId {
HubName = 'hubName',
}
export function getDefaultStringValue(id: StringSettingId): string {
switch (id) {
case StringSettingId.HubName:
return ''; // empty string will result in 'Pybricks Hub'
// istanbul ignore next: it is a programmer error if we hit this
default:
throw Error(`Bad StringSettingId: ${id}`);
}
}
src/settings/hooks.ts
+41 -1
View File
@@ -2,7 +2,9 @@
// Copyright (c) 2022 The Pybricks Authors
import { Dispatch, SetStateAction, useCallback } from 'react';
import { useLocalStorage } from 'usehooks-ts';
import { useIsFirstRender, useLocalStorage } from 'usehooks-ts';
const encoder = new TextEncoder();
// this is private type from usehooks-ts
type SetValue<T> = Dispatch<SetStateAction<T>>;
@@ -34,3 +36,41 @@ export function useSettingIsShowDocsEnabled(): {
export function useSettingFlashCurrentProgram(): [boolean, SetValue<boolean>] {
return useLocalStorage<boolean>('setting.flashCurrentProgram', false);
}
/**
* Validates the hub name.
* @param hubName The hub name.
* @returns True if the name if valid, otherwise false.
*/
function validateHubName(hubName: string): boolean {
const encoded = encoder.encode(hubName);
// Technically, the max hub name size is determined by each individual
// firmware file, so we can't check until the firmware has been selected.
// However all firmware currently have 16 bytes allocated (including zero-
// termination), so we can hard code the check here to allow notifying the
// user earlier for better UX.
return encoded.length < 16;
}
/** Hook for "hubName" setting. */
export function useSettingHubName(): {
hubName: string;
isHubNameValid: boolean;
setHubName: (value: string) => void;
} {
if (useIsFirstRender()) {
// in version 1.x, settings didn't use json format, so we have to migrate
const oldSetting = localStorage.getItem('setting.hubName');
if (oldSetting !== null && !oldSetting.startsWith('"')) {
localStorage.setItem('setting.hubName', JSON.stringify(oldSetting));
}
}
const [hubName, setHubName] = useLocalStorage('setting.hubName', '');
const isHubNameValid = validateHubName(hubName);
return { hubName, isHubNameValid, setHubName };
}
src/settings/reducers.test.ts
-67
View File
@@ -1,67 +0,0 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2021-2022 The Pybricks Authors
import { AnyAction } from 'redux';
import { didStringChange } from './actions';
import { StringSettingId } from './defaults';
import reducers from './reducers';
type State = ReturnType<typeof reducers>;
test('initial state', () => {
expect(reducers(undefined, {} as AnyAction)).toMatchInlineSnapshot(`
Object {
"hubName": "",
"isHubNameValid": true,
}
`);
});
describe('hubName', () => {
const testName = 'test name';
test('setting changed', () => {
expect(
reducers(
{ hubName: '' } as State,
didStringChange(StringSettingId.HubName, testName),
).hubName,
).toBe(testName);
});
});
describe('isHubNameValid', () => {
test('default name is ok', () => {
expect(
reducers(
undefined,
didStringChange(StringSettingId.HubName, 'Pybricks Hub'),
).isHubNameValid,
).toBe(true);
});
test('empty name is ok', () => {
expect(
reducers(undefined, didStringChange(StringSettingId.HubName, ''))
.isHubNameValid,
).toBe(true);
});
test('too long name fails', () => {
expect(
reducers(
undefined,
didStringChange(StringSettingId.HubName, 'this name is way too long'),
).isHubNameValid,
).toBe(false);
});
test('the number of bytes matter, not the number of characters', () => {
expect(
reducers(
undefined,
// Chinese characters are 3 bytes each.
didStringChange(StringSettingId.HubName, 'Pybricks 枢纽!'),
).isHubNameValid,
).toBe(false);
});
});
src/settings/reducers.ts
-45
View File
@@ -1,45 +0,0 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2021-2022 The Pybricks Authors
import { Reducer, combineReducers } from 'redux';
import { didStringChange } from './actions';
import { StringSettingId, getDefaultStringValue } from './defaults';
const encoder = new TextEncoder();
const hubName: Reducer<string> = (
state = getDefaultStringValue(StringSettingId.HubName),
action,
) => {
if (didStringChange.matches(action)) {
if (action.id === StringSettingId.HubName) {
return action.newState;
}
return state;
}
return state;
};
const isHubNameValid: Reducer<boolean> = (state = true, action) => {
if (didStringChange.matches(action)) {
if (action.id === StringSettingId.HubName) {
const encoded = encoder.encode(action.newState);
// Technically, the max hub name size is determined by each individual
// firmware file, so we can't check until the firmware has been selected.
// However all firmware currently has 16 bytes allocated (including zero-
// termination), so we can hard code the check here to allow notifying the
// user earlier for better UX.
return encoded.length < 16;
}
return state;
}
return state;
};
export default combineReducers({
hubName,
isHubNameValid,
});
src/settings/sagas.test.ts
+2 -73
View File
@@ -1,83 +1,12 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2020-2022 The Pybricks Authors
// Copyright (c) 2022 The Pybricks Authors
//
// Tests for settings sagas.
import { AsyncSaga } from '../../test';
import {
didFailToSetString,
didStringChange,
setString,
settingsToggleShowDocs,
} from './actions';
import { StringSettingId } from './defaults';
import { settingsToggleShowDocs } from './actions';
import settings from './sagas';
afterEach(() => {
jest.restoreAllMocks();
});
describe('store settings to local storage', () => {
test('failed storage', async () => {
const saga = new AsyncSaga(settings);
const testError = new Error('local storage is disabled');
const mockSetItem = jest
.spyOn(Object.getPrototypeOf(window.localStorage), 'setItem')
.mockImplementation(() => {
throw testError;
});
saga.put(setString(StringSettingId.HubName, 'test name'));
expect(mockSetItem).toHaveBeenCalled();
// raises error that storing setting didn't work
const action3 = await saga.take();
expect(action3).toEqual(didFailToSetString(StringSettingId.HubName, testError));
// but the setting is still applied anyway
const action4 = await saga.take();
expect(action4).toEqual(didStringChange(StringSettingId.HubName, 'test name'));
await saga.end();
});
});
describe('storage monitor', () => {
test('ignores other keys', async () => {
const saga = new AsyncSaga(settings);
window.dispatchEvent(
new StorageEvent('storage', {
key: 'not a setting',
storageArea: localStorage,
}),
);
// nothing happens
await saga.end();
});
test('ignores session storage', async () => {
const saga = new AsyncSaga(settings);
window.dispatchEvent(
new StorageEvent('storage', {
key: 'setting.showDocs',
newValue: 'true',
oldValue: 'false',
storageArea: sessionStorage,
}),
);
// nothing happens
await saga.end();
});
});
describe('handleToggleShowDocs', () => {
it('should toggle the showDocs setting', async () => {
const key = 'setting.showDocs';
src/settings/sagas.ts
+3 -91
View File
@@ -1,97 +1,12 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2021-2022 The Pybricks Authors
// Copyright (c) 2022 The Pybricks Authors
// This manages settings by storing them in local storage whenever the app
// request to set a setting. When local storage changes, it triggers a did
// change action that can be used by reducers to compute the new state.
import { EventChannel, eventChannel } from 'redux-saga';
import { call, fork, put, select, take, takeEvery } from 'typed-redux-saga/macro';
import { didStart } from '../app/actions';
import { RootState } from '../reducers';
import { ensureError } from '../utils';
import {
didFailToSetString,
didStringChange,
setString,
settingsToggleShowDocs,
} from './actions';
import { StringSettingId, getDefaultStringValue } from './defaults';
function createLocalStorageEventChannel(): EventChannel<StorageEvent> {
return eventChannel((emitter) => {
const handler: (e: StorageEvent) => void = (e) => {
if (e.storageArea !== localStorage) {
return;
}
emitter(e);
};
window.addEventListener('storage', handler);
// istanbul ignore next: this is not normally called
return () => window.removeEventListener('storage', handler);
});
}
function* monitorLocalStorage(): Generator {
const chan = yield* call(createLocalStorageEventChannel);
while (true) {
const event = yield* take(chan);
// only care about storage keys 'setting.*'
if (!event.key?.startsWith('setting.')) {
continue;
}
const id = event.key.replace(/^setting\./, '');
if (Object.values(StringSettingId).includes(id as StringSettingId)) {
yield* put(didStringChange(id as StringSettingId, event.newValue || ''));
continue;
}
// istanbul ignore next: should not happen normally
console.error(`Bad setting id: ${id}`);
}
}
function* loadSettings(): Generator {
for (const id of Object.values(StringSettingId)) {
const storageValue = localStorage.getItem(`setting.${id}`);
const defaultValue = getDefaultStringValue(id);
const value = storageValue === null ? defaultValue : storageValue;
if (value !== defaultValue) {
yield* put(didStringChange(id, value));
}
}
}
function* storeStringSetting(action: ReturnType<typeof setString>): Generator {
const key = `setting.${action.id}`;
const newValue = action.newState;
try {
localStorage.setItem(key, newValue);
} catch (err) {
yield* put(didFailToSetString(action.id, ensureError(err)));
}
// storage event is only raised when a value is changed externally, so we
// mimic the event when we call setItem(), whether it actually succeeded
// or not.
const oldValue = yield* select((s: RootState) => s.settings[action.id]);
if (action.newState !== oldValue) {
window.dispatchEvent(
new StorageEvent('storage', {
key,
newValue,
oldValue,
storageArea: localStorage,
}),
);
}
}
import { call, takeEvery } from 'typed-redux-saga/macro';
import { settingsToggleShowDocs } from './actions';
/**
* Hack to wire editor action to settings hook.
@@ -113,8 +28,5 @@ function* handleToggleShowDocs(): Generator {
}
export default function* (): Generator {
yield* fork(monitorLocalStorage);
yield* takeEvery(didStart, loadSettings);
yield* takeEvery(setString, storeStringSetting);
yield* takeEvery(settingsToggleShowDocs, handleToggleShowDocs);
}