settings: use react hook for showDocs setting

The moves the setting from redux state to a react hook.

src/app/App.test.tsx

@@ -1,11 +1,42 @@
 // SPDX-License-Identifier: MIT
 // Copyright (c) 2021-2022 The Pybricks Authors
 
+import { cleanup } from '@testing-library/react';
 import React from 'react';
 import { testRender } from '../../test';
 import App from './App';
 
+beforeAll(() => {
+    // this lets us use jest.spyOn with window.innerWidth
+    const defaultInnerWidth = window.innerWidth;
+    Object.defineProperty(window, 'innerWidth', {
+        get: () => defaultInnerWidth,
+    });
+});
+
+afterEach(() => {
+    cleanup();
+    jest.resetAllMocks();
+    localStorage.clear();
+});
+
 it.each([false, true])('should render', (darkMode) => {
     localStorage.setItem('usehooks-ts-dark-mode', String(darkMode));
     testRender(<App />);
 });
+
+describe('documentation pane', () => {
+    it('should show by default on large screens', () => {
+        jest.spyOn(window, 'innerWidth', 'get').mockReturnValue(1024);
+        testRender(<App />);
+        expect(document.querySelector('.pb-show-docs')).not.toBeNull();
+        expect(document.querySelector('.pb-hide-docs')).toBeNull();
+    });
+
+    it('should hide by default on small screens', () => {
+        jest.spyOn(window, 'innerWidth', 'get').mockReturnValue(800);
+        testRender(<App />);
+        expect(document.querySelector('.pb-show-docs')).toBeNull();
+        expect(document.querySelector('.pb-hide-docs')).not.toBeNull();
+    });
+});

src/app/App.tsx

@@ -8,19 +8,17 @@ import SplitterLayout from 'react-splitter-layout';
 import { useDarkMode, useLocalStorage } from 'usehooks-ts';
 import Editor, { EditorType } from '../editor/Editor';
 import Explorer from '../explorer/Explorer';
-import { useSelector } from '../reducers';
-import { toggleBoolean } from '../settings/actions';
-import { BooleanSettingId } from '../settings/defaults';
+import { settingsToggleShowDocs } from '../settings/actions';
+import { useSettingIsShowDocsEnabled } from '../settings/hooks';
 import StatusBar from '../status-bar/StatusBar';
 import Terminal from '../terminal/Terminal';
 import Toolbar from '../toolbar/Toolbar';
 import { isMacOS } from '../utils/os';
-
+import { appEditor } from './actions';
 import 'react-splitter-layout/lib/index.css';
 import './app.scss';
-import { appEditor } from './actions';
 
-const Docs: React.FunctionComponent = (_props) => {
+const Docs: React.VFC = () => {
     const dispatch = useDispatch();
 
     return (
@@ -106,7 +104,10 @@ const Docs: React.FunctionComponent = (_props) => {
                         e.key == 'd'
                     ) {
                         e.preventDefault();
-                        dispatch(toggleBoolean(BooleanSettingId.ShowDocs));
+                        // we have to use dispatch here instead of
+                        // toggleIsSettingShowDocsEnabled since this
+                        // isn't updated on state changes
+                        dispatch(settingsToggleShowDocs());
                     }
                 });
 
@@ -116,7 +117,7 @@ const Docs: React.FunctionComponent = (_props) => {
             }}
             src="static/docs/index.html"
             allowFullScreen={true}
-            title="docs"
+            role="documentation"
             width="100%"
             height="100%"
             frameBorder="none"
@@ -131,7 +132,7 @@ type AppProps = {
 
 const App: React.VoidFunctionComponent<AppProps> = ({ onEditorChanged }) => {
     const { isDarkMode } = useDarkMode();
-    const showDocs = useSelector((s): boolean => s.settings.showDocs);
+    const { isSettingShowDocsEnabled } = useSettingIsShowDocsEnabled();
     const [isDragging, setIsDragging] = useState(false);
     const dispatch = useDispatch();
 
@@ -161,7 +162,9 @@ const App: React.VoidFunctionComponent<AppProps> = ({ onEditorChanged }) => {
                 {/* need a container for SplitterLayout since it uses position: absolute */}
                 <div>
                     <SplitterLayout
-                        customClassName={showDocs ? 'pb-show-docs' : 'pb-hide-docs'}
+                        customClassName={
+                            isSettingShowDocsEnabled ? 'pb-show-docs' : 'pb-hide-docs'
+                        }
                         onDragStart={(): void => setIsDragging(true)}
                         onDragEnd={(): void => setIsDragging(false)}
                         percentage={true}

src/editor/Editor.tsx

@@ -13,8 +13,7 @@ import { useDarkMode } from 'usehooks-ts';
 import { IDisposable } from 'xterm';
 import { fileStorageWriteFile } from '../fileStorage/actions';
 import { compile } from '../mpy/actions';
-import { toggleBoolean } from '../settings/actions';
-import { BooleanSettingId } from '../settings/defaults';
+import { settingsToggleShowDocs } from '../settings/actions';
 import { isMacOS } from '../utils/os';
 import { EditorStringId } from './i18n';
 import en from './i18n.en.json';
@@ -181,7 +180,10 @@ const Editor: React.VoidFunctionComponent<EditorProps> = ({ onEditorChanged }) =
                                 id: 'pybricks.action.toggleDocs',
                                 label: i18n.translate(EditorStringId.ToggleDocs),
                                 run: () => {
-                                    dispatch(toggleBoolean(BooleanSettingId.ShowDocs));
+                                    // we have to use dispatch here instead of
+                                    // toggleIsSettingShowDocsEnabled since this
+                                    // isn't updated on state changes
+                                    dispatch(settingsToggleShowDocs());
                                 },
                                 keybindings: [
                                     monaco.KeyMod.CtrlCmd | monaco.KeyCode.KeyD,

src/settings/SettingsDrawer.test.tsx

@@ -0,0 +1,40 @@
+// 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 SettingsDrawer from './SettingsDrawer';
+
+afterEach(() => {
+    cleanup();
+    localStorage.clear();
+});
+
+describe('showDocs setting switch', () => {
+    it('should toggle setting', async () => {
+        const [settings] = testRender(
+            <SettingsDrawer isOpen={true} onClose={() => undefined} />,
+        );
+
+        const showDocs = settings.getByLabelText('Documentation');
+        expect(showDocs).toBeChecked();
+
+        userEvent.click(showDocs);
+        expect(showDocs).not.toBeChecked();
+    });
+
+    it('should have global keyboard shortcut', async () => {
+        const [settings] = testRender(
+            <SettingsDrawer isOpen={true} onClose={() => undefined} />,
+        );
+
+        const showDocs = settings.getByLabelText('Documentation');
+        expect(showDocs).toBeChecked();
+
+        userEvent.keyboard('{ctrl}d{/ctrl}');
+
+        await waitFor(() => expect(showDocs).not.toBeChecked());
+    });
+});

src/settings/SettingsDrawer.tsx

@@ -35,8 +35,9 @@ import { pseudolocalize } from '../i18n';
 import { useSelector } from '../reducers';
 import ExternalLinkIcon from '../utils/ExternalLinkIcon';
 import { isMacOS } from '../utils/os';
-import { setBoolean, setString, toggleBoolean } from './actions';
+import { setBoolean, setString } from './actions';
 import { BooleanSettingId, StringSettingId } from './defaults';
+import { useSettingIsShowDocsEnabled } from './hooks';
 import { SettingsStringId } from './i18n';
 import en from './i18n.en.json';
 import './settings.scss';
@@ -50,10 +51,14 @@ const SettingsDrawer: React.VoidFunctionComponent<SettingsProps> = ({
     isOpen,
     onClose,
 }) => {
+    const {
+        isSettingShowDocsEnabled,
+        setIsSettingShowDocsEnabled,
+        toggleIsSettingShowDocsEnabled,
+    } = useSettingIsShowDocsEnabled();
     const [isAboutDialogOpen, setIsAboutDialogOpen] = useState(false);
     const { isDarkMode, toggle: toggleDarkMode } = useDarkMode();
 
-    const showDocs = useSelector((s) => s.settings.showDocs);
     const flashCurrentProgram = useSelector((s) => s.settings.flashCurrentProgram);
     const isServiceWorkerRegistered = useSelector(
         (s) => s.app.isServiceWorkerRegistered,
@@ -83,10 +88,10 @@ const SettingsDrawer: React.VoidFunctionComponent<SettingsProps> = ({
                 label: i18n.translate(SettingsStringId.AppearanceDocumentationTooltip),
                 global: true,
                 preventDefault: true,
-                onKeyDown: () => dispatch(toggleBoolean(BooleanSettingId.ShowDocs)),
+                onKeyDown: toggleIsSettingShowDocsEnabled,
             },
         ],
-        [i18n, dispatch],
+        [i18n, toggleIsSettingShowDocsEnabled],
     );
 
     useHotkeys(hotkeys);
@@ -124,13 +129,10 @@ const SettingsDrawer: React.VoidFunctionComponent<SettingsProps> = ({
                                 label={i18n.translate(
                                     SettingsStringId.AppearanceDocumentationLabel,
                                 )}
-                                checked={showDocs}
+                                checked={isSettingShowDocsEnabled}
                                 onChange={(e) =>
-                                    dispatch(
-                                        setBoolean(
-                                            BooleanSettingId.ShowDocs,
-                                            (e.target as HTMLInputElement).checked,
-                                        ),
+                                    setIsSettingShowDocsEnabled(
+                                        (e.target as HTMLInputElement).checked,
                                     )
                                 }
                             />

src/settings/actions.ts

@@ -54,3 +54,8 @@ export const didStringChange = createAction(
         newState,
     }),
 );
+
+/** Requests to toggle the showDocs setting. */
+export const settingsToggleShowDocs = createAction(() => ({
+    type: 'editor.action.toggleShowDocs',
+}));

src/settings/defaults.ts

@@ -4,14 +4,11 @@
 // Definitions for user selectable settings.
 
 export enum BooleanSettingId {
-    ShowDocs = 'showDocs',
     FlashCurrentProgram = 'flashCurrentProgram',
 }
 
 export function getDefaultBooleanValue(id: BooleanSettingId): boolean {
     switch (id) {
-        case BooleanSettingId.ShowDocs:
-            return window.innerWidth >= 1024;
         case BooleanSettingId.FlashCurrentProgram:
             return false;
         // istanbul ignore next: it is a programmer error if we hit this

src/settings/hooks.ts

@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2022 The Pybricks Authors
+
+import { useCallback } from 'react';
+import { useLocalStorage } from 'usehooks-ts';
+
+/** Hook for "showDocs" setting. */
+export function useSettingIsShowDocsEnabled(): {
+    isSettingShowDocsEnabled: boolean;
+    setIsSettingShowDocsEnabled: (value: boolean) => void;
+    toggleIsSettingShowDocsEnabled: () => void;
+} {
+    const [isSettingShowDocsEnabled, setIsSettingShowDocsEnabled] = useLocalStorage(
+        'setting.showDocs',
+        window.innerWidth >= 1024,
+    );
+
+    const toggleIsSettingShowDocsEnabled = useCallback(
+        () => setIsSettingShowDocsEnabled((x) => !x),
+        [setIsSettingShowDocsEnabled],
+    );
+
+    return {
+        isSettingShowDocsEnabled,
+        setIsSettingShowDocsEnabled,
+        toggleIsSettingShowDocsEnabled,
+    };
+}

src/settings/reducers.test.ts

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-// Copyright (c) 2021 The Pybricks Authors
+// Copyright (c) 2021-2022 The Pybricks Authors
 
 import { AnyAction } from 'redux';
 import { didBooleanChange, didStringChange } from './actions';
@@ -14,22 +14,10 @@ test('initial state', () => {
           "flashCurrentProgram": false,
           "hubName": "",
           "isHubNameValid": true,
-          "showDocs": true,
         }
     `);
 });
 
-describe('showDocs', () => {
-    test('setting changed', () => {
-        expect(
-            reducers(
-                { showDocs: false } as State,
-                didBooleanChange(BooleanSettingId.ShowDocs, true),
-            ).showDocs,
-        ).toBe(true);
-    });
-});
-
 describe('flashCurrentProgram', () => {
     test('setting changed', () => {
         expect(

src/settings/reducers.ts

@@ -12,20 +12,6 @@ import {
 
 const encoder = new TextEncoder();
 
-const showDocs: Reducer<boolean> = (
-    state = getDefaultBooleanValue(BooleanSettingId.ShowDocs),
-    action,
-) => {
-    if (didBooleanChange.matches(action)) {
-        if (action.id === BooleanSettingId.ShowDocs) {
-            return action.newState;
-        }
-        return state;
-    }
-
-    return state;
-};
-
 const flashCurrentProgram: Reducer<boolean> = (
     state = getDefaultBooleanValue(BooleanSettingId.FlashCurrentProgram),
     action,
@@ -73,7 +59,6 @@ const isHubNameValid: Reducer<boolean> = (state = true, action) => {
 };
 
 export default combineReducers({
-    showDocs,
     flashCurrentProgram,
     hubName,
     isHubNameValid,

src/settings/sagas.test.ts

@@ -4,155 +4,21 @@
 // Tests for settings sagas.
 
 import { AsyncSaga } from '../../test';
-import { didStart } from '../app/actions';
 import {
     didBooleanChange,
-    didFailToSetBoolean,
     didFailToSetString,
     didStringChange,
     setBoolean,
     setString,
-    toggleBoolean,
+    settingsToggleShowDocs,
 } from './actions';
 import { BooleanSettingId, StringSettingId } from './defaults';
 import settings from './sagas';
 
-afterAll(() => {
+afterEach(() => {
     jest.restoreAllMocks();
 });
 
-describe('startup', () => {
-    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;
-
-            saga.put(didStart());
-
-            // does nothing
-
-            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(didStart());
-
-            // 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(didStart());
-
-            // 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(didStart());
-
-            // requests documentation to be shown
-            const action = await saga.take();
-            expect(action).toEqual(didBooleanChange(BooleanSettingId.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(didStart());
-
-            // requests documentation to be hidden
-            const action = await saga.take();
-            expect(action).toEqual(didBooleanChange(BooleanSettingId.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(didStart());
-
-            // does nothing
-
-            await saga.end();
-        });
-    });
-});
-
 describe('store settings to local storage', () => {
     test('failed storage', async () => {
         const saga = new AsyncSaga(settings);
@@ -165,21 +31,6 @@ describe('store settings to local storage', () => {
                 throw testError;
             });
 
-        saga.put(setBoolean(BooleanSettingId.ShowDocs, true));
-        expect(mockSetItem).toHaveBeenCalled();
-
-        // raises error that storing setting didn't work
-        const action1 = await saga.take();
-        expect(action1).toEqual(
-            didFailToSetBoolean(BooleanSettingId.ShowDocs, testError),
-        );
-
-        // but the setting is still applied anyway
-        const action2 = await saga.take();
-        expect(action2).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, true));
-
-        mockSetItem.mockClear();
-
         saga.put(setString(StringSettingId.HubName, 'test name'));
         expect(mockSetItem).toHaveBeenCalled();
 
@@ -194,25 +45,6 @@ describe('store settings to local storage', () => {
         await saga.end();
     });
 
-    test('showDocs', async () => {
-        const saga = new AsyncSaga(settings);
-
-        const mockSetItem = jest
-            .spyOn(Object.getPrototypeOf(window.localStorage), 'setItem')
-            .mockImplementation((key, value) => {
-                expect(key).toBe('setting.showDocs');
-                expect(value).toBe('true');
-            });
-
-        saga.put(setBoolean(BooleanSettingId.ShowDocs, true));
-        expect(mockSetItem).toHaveBeenCalled();
-
-        const action = await saga.take();
-        expect(action).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, true));
-
-        await saga.end();
-    });
-
     test('flashCurrentProgram', async () => {
         const saga = new AsyncSaga(settings);
 
@@ -253,24 +85,6 @@ describe('storage monitor', () => {
         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(BooleanSettingId.ShowDocs, true));
-
-        await saga.end();
-    });
-
     test('ignores session storage', async () => {
         const saga = new AsyncSaga(settings);
 
@@ -289,22 +103,16 @@ describe('storage monitor', () => {
     });
 });
 
-describe('toggle', () => {
-    test('showDocs', async () => {
+describe('handleToggleShowDocs', () => {
+    it('should toggle the showDocs setting', async () => {
+        const key = 'setting.showDocs';
         const saga = new AsyncSaga(settings);
 
-        const mockSetItem = jest
-            .spyOn(Object.getPrototypeOf(window.localStorage), 'setItem')
-            .mockImplementation((key, value) => {
-                expect(key).toBe('setting.showDocs');
-                expect(value).toBe('true');
-            });
+        saga.put(settingsToggleShowDocs());
+        expect(localStorage.getItem(key)).toBe('true');
 
-        saga.put(toggleBoolean(BooleanSettingId.ShowDocs));
-        expect(mockSetItem).toHaveBeenCalled();
-
-        const action = await saga.take();
-        expect(action).toEqual(didBooleanChange(BooleanSettingId.ShowDocs, true));
+        saga.put(settingsToggleShowDocs());
+        expect(localStorage.getItem(key)).toBe('false');
 
         await saga.end();
     });

src/settings/sagas.ts

@@ -17,6 +17,7 @@ import {
     didStringChange,
     setBoolean,
     setString,
+    settingsToggleShowDocs,
     toggleBoolean,
 } from './actions';
 import {
@@ -157,10 +158,30 @@ function* storeStringSetting(action: ReturnType<typeof setString>): Generator {
     }
 }
 
+/**
+ * Hack to wire editor action to settings hook.
+ *
+ * There are a few places where using toggleIsSettingShowDocsEnabled() doesn't
+ * work because it needs to be called from outside of a React component that
+ * doesn't get updated when state changes.
+ */
+function* handleToggleShowDocs(): Generator {
+    // HACK: This depends on the implementation detail that
+    // useSettingIsShowDocsEnabled() uses useLocalStorage() internally.
+    yield* call(() => {
+        const key = 'setting.showDocs';
+        const oldValue = localStorage.getItem(key);
+        const newValue = JSON.stringify(!(oldValue && JSON.parse(oldValue)));
+        localStorage.setItem(key, newValue);
+        window.dispatchEvent(new Event('local-storage'));
+    });
+}
+
 export default function* (): Generator {
     yield* fork(monitorLocalStorage);
     yield* takeEvery(didStart, loadSettings);
     yield* takeEvery(setBoolean, storeBooleanSetting);
     yield* takeEvery(toggleBoolean, toggleBooleanSetting);
     yield* takeEvery(setString, storeStringSetting);
+    yield* takeEvery(settingsToggleShowDocs, handleToggleShowDocs);
 }

src/setupTests.ts

@@ -1,8 +1,15 @@
+// SPDX-License-Identifier: MIT
+// Copyright (c) 2020-2022 The Pybricks Authors
+
 // jest-dom adds custom jest matchers for asserting on DOM nodes.
 // allows you to do things like:
 // expect(element).toHaveTextContent(/react/i)
 // learn more: https://github.com/testing-library/jest-dom
 import '@testing-library/jest-dom/extend-expect';
+import {
+    KeyCodes,
+    Modifiers,
+} from '@blueprintjs/core/lib/cjs/components/hotkeys/hotkeyParser';
 // @ts-expect-error no typings
 import matchMediaPolyfill from 'mq-polyfill';
 
@@ -18,3 +25,42 @@ window.resizeTo = function resizeTo(width, height) {
         outerHeight: height,
     }).dispatchEvent(new this.Event('resize'));
 };
+
+// HACK: work around https://github.com/palantir/blueprint/issues/4165
+// userEvent.keyboard does not set which, so we have to do a reverse lookup
+// using the blueprintjs keymap.
+
+// handle cases that are not simple lower case conversion
+const specialCases: Record<string, string> = {
+    Control: 'ctrl',
+    ' ': 'space',
+    ArrowDown: 'down',
+    ArrowLeft: 'left',
+    ArrowRight: 'right',
+    ArrowUp: 'up',
+    Delete: 'del',
+    Insert: 'ins',
+    Escape: 'esc',
+};
+
+function addWhichToKeyboardEvent(e: KeyboardEvent) {
+    const blueprintsKeyName = specialCases[e.key] ?? e.key.toLowerCase();
+    let which = 0;
+
+    for (const [k, v] of Object.entries(KeyCodes).concat(Object.entries(Modifiers))) {
+        if (v === blueprintsKeyName) {
+            which = Number(k);
+            break;
+        }
+    }
+
+    if (which === 0) {
+        console.warn('unsupported key:', e.key);
+    }
+
+    Object.defineProperty(e, 'which', { value: which });
+}
+
+document.addEventListener('keydown', addWhichToKeyboardEvent);
+document.addEventListener('keypress', addWhichToKeyboardEvent);
+document.addEventListener('keyup', addWhichToKeyboardEvent);