convert settings to do/did action pattern

src/actions/settings.ts

@@ -2,28 +2,59 @@
 // Copyright (c) 2021 The Pybricks Authors
 
 import { Action } from 'redux';
+import { SettingId } from '../settings';
 
 /** Actions related to settings. */
 export enum SettingsActionType {
-    ToggleDocs = 'settings.action.toggleDocs',
-    ToggleDarkMode = 'settings.action.toggleDarkMode',
+    SetBoolean = 'settings.action.setBoolean',
+    DidFailToSetBoolean = 'settings.action.didFailToSetBoolean',
+    DidBooleanChange = 'settings.action.didBooleanChange',
 }
 
-/** Action to toggle show docs setting. */
-export type SettingsToggleDocsAction = Action<SettingsActionType.ToggleDocs>;
+type SettingInfo<T> = {
+    /** The ID of the setting. */
+    id: SettingId;
+    /** The new state for the setting. */
+    newState: T;
+};
 
-/** Toggles show docs setting on or off. */
-export function toggleDocs(): SettingsToggleDocsAction {
-    return { type: SettingsActionType.ToggleDocs };
+/** Action to set/store a setting. */
+export type SettingsSetBooleanAction = Action<SettingsActionType.SetBoolean> &
+    SettingInfo<boolean>;
+
+/** Creates an action to set/store a setting. */
+export function setBoolean(id: SettingId, newState: boolean): SettingsSetBooleanAction {
+    return { type: SettingsActionType.SetBoolean, id, newState };
 }
 
-/** Action to toggle dark mode setting. */
-export type SettingsToggleDarkModeAction = Action<SettingsActionType.ToggleDarkMode>;
+/** Action that indicates setting/storing a setting failed. */
+export type SettingsDidFailToSetBooleanAction = Action<SettingsActionType.DidFailToSetBoolean> & {
+    id: SettingId;
+    err: Error;
+};
 
-/** Toggles dark mode setting on or off. */
-export function toggleDarkMode(): SettingsToggleDarkModeAction {
-    return { type: SettingsActionType.ToggleDarkMode };
+/** Creates an action indicating that setting/storing a setting failed. */
+export function didFailToSetBoolean(
+    id: SettingId,
+    err: Error,
+): SettingsDidFailToSetBooleanAction {
+    return { type: SettingsActionType.DidFailToSetBoolean, id, err };
 }
 
-/** common type for all settings actions. */
-export type SettingsAction = SettingsToggleDocsAction | SettingsToggleDarkModeAction;
+/** Action that indicates a stored boolean setting value changed. */
+export type SettingsDidBooleanChangeAction = Action<SettingsActionType.DidBooleanChange> &
+    SettingInfo<boolean>;
+
+/** Creates an action that indicates a stored boolean setting value changed. */
+export function didBooleanChange(
+    id: SettingId,
+    newState: boolean,
+): SettingsDidBooleanChangeAction {
+    return { type: SettingsActionType.DidBooleanChange, id, newState };
+}
+
+/** Common type for all settings actions. */
+export type SettingsAction =
+    | SettingsSetBooleanAction
+    | SettingsDidFailToSetBooleanAction
+    | SettingsDidBooleanChangeAction;

src/components/SettingsDrawer.tsx

@@ -7,8 +7,9 @@ import React from 'react';
 import { connect } from 'react-redux';
 import { Action, Dispatch } from '../actions';
 import { closeSettings } from '../actions/app';
-import { toggleDarkMode, toggleDocs } from '../actions/settings';
+import { setBoolean } from '../actions/settings';
 import { RootState } from '../reducers';
+import { SettingId } from '../settings';
 import { SettingsStringId } from './settings-i18n';
 import en from './settings-i18n.en.json';
 
@@ -22,8 +23,8 @@ type StateProps = {
 
 type DispatchProps = {
     onClose: () => void;
-    onShowDocsChanged: () => void;
-    onDarkModeChanged: () => void;
+    onShowDocsChanged: (checked: boolean) => void;
+    onDarkModeChanged: (checked: boolean) => void;
 };
 
 type SettingsProps = StateProps & DispatchProps & WithI18nProps;
@@ -55,7 +56,11 @@ class SettingsDrawer extends React.PureComponent<SettingsProps> {
                             )}
                             large={true}
                             checked={showDocs}
-                            onChange={() => onShowDocsChanged()}
+                            onChange={(e) =>
+                                onShowDocsChanged(
+                                    (e.target as HTMLInputElement).checked,
+                                )
+                            }
                         />
                         <Switch
                             label={i18n.translate(
@@ -63,7 +68,11 @@ class SettingsDrawer extends React.PureComponent<SettingsProps> {
                             )}
                             large={true}
                             checked={darkMode}
-                            onChange={() => onDarkModeChanged()}
+                            onChange={(e) =>
+                                onDarkModeChanged(
+                                    (e.target as HTMLInputElement).checked,
+                                )
+                            }
                         />
                     </FormGroup>
                 </div>
@@ -80,8 +89,10 @@ const mapStateToProps = (state: RootState): StateProps => ({
 
 const mapDispatchToProps = (dispatch: Dispatch): DispatchProps => ({
     onClose: (): Action => dispatch(closeSettings()),
-    onShowDocsChanged: (): Action => dispatch(toggleDocs()),
-    onDarkModeChanged: (): Action => dispatch(toggleDarkMode()),
+    onShowDocsChanged: (checked): Action =>
+        dispatch(setBoolean(SettingId.ShowDocs, checked)),
+    onDarkModeChanged: (checked): Action =>
+        dispatch(setBoolean(SettingId.DarkMode, checked)),
 });
 
 export default connect(

src/index.tsx

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-// Copyright (c) 2020 The Pybricks Authors
+// Copyright (c) 2020-2021 The Pybricks Authors
 
 import { ResizeSensor } from '@blueprintjs/core';
 import { I18nContext, I18nManager } from '@shopify/react-i18n';
@@ -32,6 +32,20 @@ const store = createStore(
     applyMiddleware(sagaMiddleware, loggerMiddleware),
 );
 
+// Hook in blueprints dark mode class to setting
+let oldDarkMode = false;
+store.subscribe(() => {
+    const newDarkMode = store.getState().settings.darkMode;
+    if (newDarkMode !== oldDarkMode) {
+        if (newDarkMode) {
+            document.body.classList.add('bp3-dark');
+        } else {
+            document.body.classList.remove('bp3-dark');
+        }
+        oldDarkMode = newDarkMode;
+    }
+});
+
 sagaMiddleware.run(rootSaga);
 
 ReactDOM.render(

src/reducers/settings.ts

@@ -4,25 +4,38 @@
 import { Reducer, combineReducers } from 'redux';
 import { Action } from '../actions';
 import { SettingsActionType } from '../actions/settings';
+import { SettingId, getDefaultBooleanValue } from '../settings';
 
 export interface SettingsState {
     readonly darkMode: boolean;
     readonly showDocs: boolean;
 }
 
-const darkMode: Reducer<boolean, Action> = (state = false, action) => {
+const darkMode: Reducer<boolean, Action> = (
+    state = getDefaultBooleanValue(SettingId.DarkMode),
+    action,
+) => {
     switch (action.type) {
-        case SettingsActionType.ToggleDarkMode:
-            return !state;
+        case SettingsActionType.DidBooleanChange:
+            if (action.id === SettingId.DarkMode) {
+                return action.newState;
+            }
+            return state;
         default:
             return state;
     }
 };
 
-const showDocs: Reducer<boolean, Action> = (state = false, action) => {
+const showDocs: Reducer<boolean, Action> = (
+    state = getDefaultBooleanValue(SettingId.ShowDocs),
+    action,
+) => {
     switch (action.type) {
-        case SettingsActionType.ToggleDocs:
-            return !state;
+        case SettingsActionType.DidBooleanChange:
+            if (action.id === SettingId.ShowDocs) {
+                return action.newState;
+            }
+            return state;
         default:
             return state;
     }

src/sagas/settings.test.ts

@@ -1,11 +1,13 @@
 // SPDX-License-Identifier: MIT
 // Copyright (c) 2020-2021 The Pybricks Authors
-// File: sagas/app.test.ts
-// Tests for app sagas.
+// File: sagas/settings.test.ts
+// Tests for settings sagas.
 
 import { AsyncSaga } from '../../test';
 import { startup } from '../actions/app';
-import { SettingsActionType, toggleDarkMode, toggleDocs } from '../actions/settings';
+import { didBooleanChange, didFailToSetBoolean, setBoolean } from '../actions/settings';
+import { SettingsState } from '../reducers/settings';
+import { SettingId } from '../settings';
 import settings from './settings';
 
 afterAll(() => {
@@ -13,107 +15,316 @@ afterAll(() => {
 });
 
 describe('startup', () => {
-    test('with large screen', async () => {
-        const saga = new AsyncSaga(settings);
+    describe('showDocs', () => {
+        test('with large screen and no value set', async () => {
+            const saga = new AsyncSaga(settings);
 
-        jest.spyOn(
-            Object.getPrototypeOf(window.localStorage),
-            'getItem',
-        ).mockReturnValue(null);
-        innerWidth = 1024;
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockReturnValue(null);
+            innerWidth = 1024;
 
-        saga.put(startup());
+            saga.put(startup());
 
-        // toggles documentation to be visible
-        const toggleDocsAction = await saga.take();
-        expect(toggleDocsAction.type).toBe(SettingsActionType.ToggleDocs);
+            // does nothing
 
-        await saga.end();
+            await saga.end();
+        });
+
+        test('with small screen and no value set', async () => {
+            const saga = new AsyncSaga(settings);
+
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockReturnValue(null);
+            innerWidth = 800;
+
+            saga.put(startup());
+
+            // does nothing
+
+            await saga.end();
+        });
+
+        test('with large screen and stored value "true"', async () => {
+            const saga = new AsyncSaga(settings);
+
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockImplementation((key) => {
+                switch (key) {
+                    case 'setting.showDocs':
+                        return 'true';
+                    default:
+                        return null;
+                }
+            });
+            innerWidth = 1024;
+
+            saga.put(startup());
+
+            // does nothing
+
+            await saga.end();
+        });
+
+        test('with small screen and stored value "true"', async () => {
+            const saga = new AsyncSaga(settings);
+
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockImplementation((key) => {
+                switch (key) {
+                    case 'setting.showDocs':
+                        return 'true';
+                    default:
+                        return null;
+                }
+            });
+            innerWidth = 800;
+
+            saga.put(startup());
+
+            // requests documentation to be shown
+            const action = await saga.take();
+            expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, true));
+
+            await saga.end();
+        });
+
+        test('with large screen stored value "false"', async () => {
+            const saga = new AsyncSaga(settings);
+
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockImplementation((key) => {
+                switch (key) {
+                    case 'setting.showDocs':
+                        return 'false';
+                    default:
+                        return null;
+                }
+            });
+            innerWidth = 1024;
+
+            saga.put(startup());
+
+            // requests documentation to be hidden
+            const action = await saga.take();
+            expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, false));
+
+            await saga.end();
+        });
+
+        test('with small screen stored value "false"', async () => {
+            const saga = new AsyncSaga(settings);
+
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockImplementation((key) => {
+                switch (key) {
+                    case 'setting.showDocs':
+                        return 'false';
+                    default:
+                        return null;
+                }
+            });
+            innerWidth = 800;
+
+            saga.put(startup());
+
+            // does nothing
+
+            await saga.end();
+        });
     });
 
-    test('with small screen', async () => {
-        const saga = new AsyncSaga(settings);
+    describe('darkMode', () => {
+        test('with no value set', async () => {
+            const saga = new AsyncSaga(settings);
 
-        jest.spyOn(
-            Object.getPrototypeOf(window.localStorage),
-            'getItem',
-        ).mockReturnValue(null);
-        innerWidth = 800;
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockReturnValue(null);
 
-        saga.put(startup());
+            saga.put(startup());
 
-        // does nothing
+            // does nothing
 
-        await saga.end();
-    });
+            await saga.end();
+        });
 
-    test('with stored value "true"', async () => {
-        const saga = new AsyncSaga(settings);
+        test('with value set to true', async () => {
+            const saga = new AsyncSaga(settings);
 
-        jest.spyOn(
-            Object.getPrototypeOf(window.localStorage),
-            'getItem',
-        ).mockReturnValue('{"showDocs":true}');
-        innerWidth = 800;
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockImplementation((key) => {
+                switch (key) {
+                    case 'setting.darkMode':
+                        return 'true';
+                    default:
+                        return null;
+                }
+            });
 
-        saga.put(startup());
+            saga.put(startup());
 
-        // toggles documentation to be visible
-        const toggleDocsAction = await saga.take();
-        expect(toggleDocsAction.type).toBe(SettingsActionType.ToggleDocs);
+            // requests to enable dark mode
+            const action = await saga.take();
+            expect(action).toEqual(didBooleanChange(SettingId.DarkMode, true));
 
-        await saga.end();
-    });
+            await saga.end();
+        });
 
-    test('with stored value "false"', async () => {
-        const saga = new AsyncSaga(settings);
+        test('with value set to false', async () => {
+            const saga = new AsyncSaga(settings);
 
-        jest.spyOn(
-            Object.getPrototypeOf(window.localStorage),
-            'getItem',
-        ).mockReturnValue('{"showDocs":false}');
-        innerWidth = 1024;
+            jest.spyOn(
+                Object.getPrototypeOf(window.localStorage),
+                'getItem',
+            ).mockImplementation((key) => {
+                switch (key) {
+                    case 'setting.darkMode':
+                        return 'false';
+                    default:
+                        return null;
+                }
+            });
 
-        saga.put(startup());
+            saga.put(startup());
 
-        // does nothing
+            // does nothing
 
-        await saga.end();
+            await saga.end();
+        });
     });
 });
 
 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.setState({ settings: { showDocs: false } as SettingsState });
+        saga.put(setBoolean(SettingId.ShowDocs, true));
+        expect(mockSetItem).toHaveBeenCalled();
+
+        // raises error that storing setting didn't work
+        const action1 = await saga.take();
+        expect(action1).toEqual(didFailToSetBoolean(SettingId.ShowDocs, testError));
+
+        // but the setting is still applied anyway
+        const action2 = await saga.take();
+        expect(action2).toEqual(didBooleanChange(SettingId.ShowDocs, true));
+
+        await saga.end();
+    });
+
     test('showDocs', async () => {
         const saga = new AsyncSaga(settings);
 
-        // NOTE: we aren't testing reducers here, so value doesn't change
-        // even though we call the toggle function
         const mockSetItem = jest
             .spyOn(Object.getPrototypeOf(window.localStorage), 'setItem')
-            .mockImplementation((_key, value) =>
-                expect(value).toBe('{"darkMode":false,"showDocs":true}'),
-            );
-        saga.setState({ settings: { darkMode: false, showDocs: true } });
-        saga.put(toggleDocs());
+            .mockImplementation((key, value) => {
+                expect(key).toBe('setting.showDocs');
+                expect(value).toBe('true');
+            });
+
+        saga.setState({ settings: { showDocs: false } as SettingsState });
+        saga.put(setBoolean(SettingId.ShowDocs, true));
         expect(mockSetItem).toHaveBeenCalled();
 
+        const action = await saga.take();
+        expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, true));
+
         await saga.end();
     });
 
     test('darkMode', async () => {
         const saga = new AsyncSaga(settings);
 
-        // NOTE: we aren't testing reducers here, so value doesn't change
-        // even though we call the toggle function
         const mockSetItem = jest
             .spyOn(Object.getPrototypeOf(window.localStorage), 'setItem')
-            .mockImplementation((_key, value) =>
-                expect(value).toBe('{"darkMode":true,"showDocs":false}'),
-            );
-        saga.setState({ settings: { darkMode: true, showDocs: false } });
-        saga.put(toggleDarkMode());
+            .mockImplementation((key, value) => {
+                expect(key).toBe('setting.darkMode');
+                expect(value).toBe('false');
+            });
+
+        saga.setState({ settings: { darkMode: true } as SettingsState });
+        saga.put(setBoolean(SettingId.DarkMode, false));
         expect(mockSetItem).toHaveBeenCalled();
 
+        const action = await saga.take();
+        expect(action).toEqual(didBooleanChange(SettingId.DarkMode, false));
+
+        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('puts action when setting changes', async () => {
+        const saga = new AsyncSaga(settings);
+
+        window.dispatchEvent(
+            new StorageEvent('storage', {
+                key: 'setting.showDocs',
+                newValue: 'true',
+                oldValue: 'false',
+                storageArea: localStorage,
+            }),
+        );
+
+        const action = await saga.take();
+        expect(action).toEqual(didBooleanChange(SettingId.ShowDocs, true));
+
+        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();
     });
 });

src/sagas/settings.ts

@@ -1,45 +1,106 @@
-import { put, select, takeEvery } from 'redux-saga/effects';
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2021 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 'redux-saga/effects';
 import { AppActionType } from '../actions/app';
-import { SettingsActionType, toggleDarkMode, toggleDocs } from '../actions/settings';
+import {
+    SettingsActionType,
+    SettingsSetBooleanAction,
+    didBooleanChange,
+    didFailToSetBoolean,
+} from '../actions/settings';
 import { RootState } from '../reducers';
-import { SettingsState } from '../reducers/settings';
+import { SettingId, getDefaultBooleanValue } from '../settings';
+
+function stringToBoolean(value: string): boolean {
+    return value.toLowerCase().match(/(true|yes|1)/) !== null;
+}
+
+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,
+    )) as EventChannel<StorageEvent>;
+
+    while (true) {
+        const event = (yield take(chan)) as StorageEvent;
+
+        // only care about storage keys 'setting.*'
+        if (!event.key?.startsWith('setting.')) {
+            continue;
+        }
+
+        const id = event.key.replace(/^setting\./, '') as SettingId;
+
+        // istanbul ignore if: should not happen normally
+        if (!Object.values(SettingId).includes(id)) {
+            console.error(`Bad setting id: ${id}`);
+            continue;
+        }
+
+        yield put(didBooleanChange(id, stringToBoolean(event.newValue || 'false')));
+    }
+}
 
 function* loadSettings(): Generator {
-    const settingsString = localStorage.getItem('settings') || '{}';
-    const settings = JSON.parse(settingsString) as SettingsState;
+    for (const id of Object.values(SettingId)) {
+        const storageValue = localStorage.getItem(`setting.${id}`);
+        const defaultValue = getDefaultBooleanValue(id);
+        const value =
+            storageValue === null ? defaultValue : stringToBoolean(storageValue);
 
-    // TODO: there has to be a better way to initialize app state from settings
-
-    if (
-        settings.showDocs === undefined
-            ? window.innerWidth >= 1024
-            : settings.showDocs === true
-    ) {
-        yield put(toggleDocs());
-    }
-
-    if (settings.darkMode) {
-        yield put(toggleDarkMode());
+        if (value !== defaultValue) {
+            yield put(didBooleanChange(id, value));
+        }
     }
 }
 
-function* saveSettings(): Generator {
-    const settings = (yield select((s: RootState) => s.settings)) as SettingsState;
-    localStorage.setItem('settings', JSON.stringify(settings));
-}
+function* storeSetting(action: SettingsSetBooleanAction): Generator {
+    const key = `setting.${action.id}`;
+    const newValue = String(action.newState);
 
-// TODO: this should really be part of component, not saga
-function* updateDarkModeClass(): Generator {
-    const darkMode = (yield select((s: RootState) => s.settings.darkMode)) as boolean;
-    if (darkMode) {
-        document.body.classList.add('bp3-dark');
-    } else {
-        document.body.classList.remove('bp3-dark');
+    try {
+        localStorage.setItem(key, newValue);
+    } catch (err) {
+        yield put(didFailToSetBoolean(action.id, 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 oldState = (yield select((s: RootState) => s.settings[action.id])) as boolean;
+    if (action.newState !== oldState) {
+        window.dispatchEvent(
+            new StorageEvent('storage', {
+                key,
+                newValue,
+                oldValue: String(oldState),
+                storageArea: localStorage,
+            }),
+        );
     }
 }
 
 export default function* (): Generator {
+    yield fork(monitorLocalStorage);
     yield takeEvery(AppActionType.Startup, loadSettings);
-    yield takeEvery(Object.values(SettingsActionType), saveSettings);
-    yield takeEvery(SettingsActionType.ToggleDarkMode, updateDarkModeClass);
+    yield takeEvery(SettingsActionType.SetBoolean, storeSetting);
 }

src/settings/index.ts

@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2021 The Pybricks Authors
+
+// Definitions for user settings.
+
+export enum SettingId {
+    ShowDocs = 'showDocs',
+    DarkMode = 'darkMode',
+}
+
+export function getDefaultBooleanValue(id: SettingId): boolean {
+    switch (id) {
+        case SettingId.ShowDocs:
+            return window.innerWidth >= 1024;
+        case SettingId.DarkMode:
+            return false;
+        default:
+            throw Error(`Bad setting id: ${id}`);
+    }
+}