diff --git a/src/app/App.test.tsx b/src/app/App.test.tsx index c813a38e..2f15a31f 100644 --- a/src/app/App.test.tsx +++ b/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(); }); + +describe('documentation pane', () => { + it('should show by default on large screens', () => { + jest.spyOn(window, 'innerWidth', 'get').mockReturnValue(1024); + testRender(); + 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(); + expect(document.querySelector('.pb-show-docs')).toBeNull(); + expect(document.querySelector('.pb-hide-docs')).not.toBeNull(); + }); +}); diff --git a/src/app/App.tsx b/src/app/App.tsx index 49c1041d..c049f48d 100644 --- a/src/app/App.tsx +++ b/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 = ({ 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 = ({ onEditorChanged }) => { {/* need a container for SplitterLayout since it uses position: absolute */}
setIsDragging(true)} onDragEnd={(): void => setIsDragging(false)} percentage={true} diff --git a/src/editor/Editor.tsx b/src/editor/Editor.tsx index 4cf7c006..20884feb 100644 --- a/src/editor/Editor.tsx +++ b/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 = ({ 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, diff --git a/src/settings/SettingsDrawer.test.tsx b/src/settings/SettingsDrawer.test.tsx new file mode 100644 index 00000000..7fe01fb4 --- /dev/null +++ b/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( + 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( + undefined} />, + ); + + const showDocs = settings.getByLabelText('Documentation'); + expect(showDocs).toBeChecked(); + + userEvent.keyboard('{ctrl}d{/ctrl}'); + + await waitFor(() => expect(showDocs).not.toBeChecked()); + }); +}); diff --git a/src/settings/SettingsDrawer.tsx b/src/settings/SettingsDrawer.tsx index 379d46e2..9cb24c07 100644 --- a/src/settings/SettingsDrawer.tsx +++ b/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 = ({ 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 = ({ 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 = ({ 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, ) } /> diff --git a/src/settings/actions.ts b/src/settings/actions.ts index 80077de4..5228cc66 100644 --- a/src/settings/actions.ts +++ b/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', +})); diff --git a/src/settings/defaults.ts b/src/settings/defaults.ts index a7f7b99a..1d353ba6 100644 --- a/src/settings/defaults.ts +++ b/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 diff --git a/src/settings/hooks.ts b/src/settings/hooks.ts new file mode 100644 index 00000000..ea7521f3 --- /dev/null +++ b/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, + }; +} diff --git a/src/settings/reducers.test.ts b/src/settings/reducers.test.ts index 1c2aaa82..4201464e 100644 --- a/src/settings/reducers.test.ts +++ b/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( diff --git a/src/settings/reducers.ts b/src/settings/reducers.ts index 9db878a4..44a8fc3a 100644 --- a/src/settings/reducers.ts +++ b/src/settings/reducers.ts @@ -12,20 +12,6 @@ import { const encoder = new TextEncoder(); -const showDocs: Reducer = ( - state = getDefaultBooleanValue(BooleanSettingId.ShowDocs), - action, -) => { - if (didBooleanChange.matches(action)) { - if (action.id === BooleanSettingId.ShowDocs) { - return action.newState; - } - return state; - } - - return state; -}; - const flashCurrentProgram: Reducer = ( state = getDefaultBooleanValue(BooleanSettingId.FlashCurrentProgram), action, @@ -73,7 +59,6 @@ const isHubNameValid: Reducer = (state = true, action) => { }; export default combineReducers({ - showDocs, flashCurrentProgram, hubName, isHubNameValid, diff --git a/src/settings/sagas.test.ts b/src/settings/sagas.test.ts index 111f7b95..adb1e741 100644 --- a/src/settings/sagas.test.ts +++ b/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(); }); diff --git a/src/settings/sagas.ts b/src/settings/sagas.ts index dcb1f1b0..a5b0ed4c 100644 --- a/src/settings/sagas.ts +++ b/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): 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); } diff --git a/src/setupTests.ts b/src/setupTests.ts index 6d49337e..fde67d37 100644 --- a/src/setupTests.ts +++ b/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 = { + 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);