diff --git a/docs/pages/connect-your-client/teleport-connect.mdx b/docs/pages/connect-your-client/teleport-connect.mdx index 62eafe1b3f0d8..5d73892b4b383 100644 --- a/docs/pages/connect-your-client/teleport-connect.mdx +++ b/docs/pages/connect-your-client/teleport-connect.mdx @@ -158,27 +158,74 @@ installation directory to the `Path` user environment variable. +## Configuration + +Teleport Connect can be configured by editing the `app_config.json` file, which it creates on first launch. +To open it, click the More Options icon `⋮` -> Open Config File. +The file will open in your default editor. + + + Any changes to the config file will take effect at the next launch. + + +Below is the list of the supported config properties. + + +| Property | Default | Description | +|-------------------------------|----------------------------------------------------------------------------------------------------------------------|------------------------------------------------------------------------| +| `usageReporting.enabled` | `false` | Enables collecting anonymous usage data (see [Telemetry](#telemetry)). | +| `keymap.tab1` - `keymap.tab9` | `Command+1` - `Command+9` on macOS
`Ctrl+1` - `Ctrl+9` on Windows
`Alt+1` - `Alt+9` on Linux | Shortcut to open tab 1–9. | +| `keymap.closeTab` | `Command+W` on macOS
`Ctrl+W` on Windows/Linux | Shortcut to close a tab. | +| `keymap.newTab` | `Command+T` on macOS
`Ctrl+T` on Windows/Linux | Shortcut to open a new tab. | +| `keymap.previousTab` | `Shift+Command+Tab` on macOS
`Ctrl+Shift+Tab` on Windows/Linux | Shortcut to go to the previous tab. | +| `keymap.nextTab` | `Command+Tab` on macOS
`Ctrl+Tab` on Windows/Linux | Shortcut to go to the next tab. | +| `keymap.openConnections` | `Command+P` on macOS
`Ctrl+P` on Windows/Linux | Shortcut to open the connection list. | +| `keymap.openClusters` | `Command+E` on macOS
`Ctrl+E` on Windows/Linux | Shortcut to open the cluster selector. | +| `keymap.openProfiles` | `Command+I` on macOS
`Ctrl+I` on Windows/Linux | Shortcut to open the profile selector. | +| `keymap.openCommandBar` | `Command+K` on macOS
`Ctrl+K` on Windows/Linux | Shortcut to open the command bar. | +| `terminal.fontFamily` | `Menlo, Monaco, monospace` on macOS
`Consolas, monospace` on Windows
`'Droid Sans Mono', monospace` on Linux | Font family for the terminal. | +| `terminal.fontSize` | 15 | Font size for the terminal. | + + + The additional `$schema` property present in the config file allows text editors to provide autocompletion. + It should not be modified. + + +### Configuring keyboard shortcuts + +A valid shortcut contains at least one modifier and a single key code, for example `Shift+Tab`. +Function keys such as `F1` do not require a modifier. +Modifiers and a key code must be combined by the `+` character. + +Available modifiers: + +- `Control`, `Option`, `Shift`, `Command` on macOS. +- `Ctrl`, `Alt`, `Shift` on Windows and Linux. + +Available key codes: +- `0` to `9` +- `A` to `Z` +- `F1` to `F24` +- `,`, `.`, `/`, `\`, `` ` ``, `-`, `=`, `;`, `'`, `[`, `]` +- `Space`, `Tab`, `CapsLock`, `NumLock`, `ScrollLock`, `Backspace`, `Delete`, `Insert`, `Enter`, `Up`, `Down`, `Left`, `Right`, `Home`, `End`, `PageUp`, `PageDown`, `Escape`, `IntlBackslash` + + ## Telemetry (!docs/pages/includes/teleport-connect-telemetry.mdx!) ### Disabling telemetry -If you initially agreed to share telemetry data, but now want to opt out, you need to update the `app_config.json` file: +If you initially agreed to share telemetry data, but now want to opt out, you need to set `usageReporting.enabled` in the config to `false` (see [Configuration](#configuration)): ```json "usageReporting.enabled": false ``` - - -The Teleport Connect configuration file is located at `~/Library/Application Support/Teleport Connect/app_config.json`. - - -The Teleport Connect configuration file is located at `~/.config/Teleport Connect/app_config.json`. - - -The Teleport Connect configuration file is located at `C:\Users\%UserName%\AppData\Roaming\Teleport Connect\app_config.json`. - - The changes will take effect at the next launch. diff --git a/web/packages/shared/components/Notification/Notification.story.tsx b/web/packages/shared/components/Notification/Notification.story.tsx index 8f166c1446425..08813e3cd8adb 100644 --- a/web/packages/shared/components/Notification/Notification.story.tsx +++ b/web/packages/shared/components/Notification/Notification.story.tsx @@ -26,7 +26,13 @@ export default { export const Notifications = () => { return ( - +
props.theme.space[8]}px; + grid-template-columns: auto auto auto; + `} + > { isAutoRemovable={false} /> - + + + theme.colors.info} + onRemove={() => {}} + isAutoRemovable={false} + /> + theme.colors.warning} + onRemove={() => {}} + isAutoRemovable={false} + /> + theme.colors.danger} + onRemove={() => {}} + isAutoRemovable={false} + /> + + + + theme.colors.info} + onRemove={() => {}} + isAutoRemovable={false} + /> + theme.colors.warning} + onRemove={() => {}} + isAutoRemovable={false} + /> + theme.colors.danger} + onRemove={() => {}} + isAutoRemovable={false} + /> + +
); }; diff --git a/web/packages/shared/components/Notification/Notification.tsx b/web/packages/shared/components/Notification/Notification.tsx index 5e9853a6e68de..a66bafaeff522 100644 --- a/web/packages/shared/components/Notification/Notification.tsx +++ b/web/packages/shared/components/Notification/Notification.tsx @@ -16,16 +16,20 @@ limitations under the License. import React, { useEffect, useRef, useState } from 'react'; import styled, { css, useTheme } from 'styled-components'; -import { ButtonIcon, Flex, Text } from 'design'; +import { ButtonIcon, Flex, Link, Text } from 'design'; import { Close } from 'design/Icon'; import type { NotificationItem, NotificationItemContent } from './types'; interface NotificationProps { item: NotificationItem; + onRemove(): void; + Icon: React.ElementType; + getColor(theme): string; + isAutoRemovable: boolean; autoRemoveDurationMs?: number; // Workaround until `styled` gets types. @@ -171,13 +175,46 @@ function getRenderedContent( color="text.secondary" css={longerTextCss} > + {content.list && } {content.description} + {content.link && ( + + {content.link.text} + + )}
); } } +function List(props: { items: string[] }) { + return ( + + ); +} + const textCss = css` line-height: 20px; overflow-wrap: break-word; diff --git a/web/packages/shared/components/Notification/types.ts b/web/packages/shared/components/Notification/types.ts index ea009cad63018..6c936d4509011 100644 --- a/web/packages/shared/components/Notification/types.ts +++ b/web/packages/shared/components/Notification/types.ts @@ -20,6 +20,14 @@ export interface NotificationItem { id: string; } -export type NotificationItemContent = - | string - | { title: string; description: string }; +export type NotificationItemContent = string | NotificationItemObjectContent; + +export type NotificationItemObjectContent = { + title: string; + list?: string[]; + description?: string; + link?: { + href: string; + text: string; + }; +}; diff --git a/web/packages/teleterm/package.json b/web/packages/teleterm/package.json index fb7fe0286bbd0..d865d50a1c4ee 100644 --- a/web/packages/teleterm/package.json +++ b/web/packages/teleterm/package.json @@ -56,7 +56,8 @@ "winston": "^3.3.3", "xterm": "^5.0.0", "xterm-addon-fit": "^0.7.0", - "zod": "^3.20.0" + "zod": "^3.21.2", + "zod-to-json-schema": "^3.20.4" }, "productName": "Teleport Connect" } diff --git a/web/packages/teleterm/src/main.ts b/web/packages/teleterm/src/main.ts index 0879c270689fc..4d31f54ef14d0 100644 --- a/web/packages/teleterm/src/main.ts +++ b/web/packages/teleterm/src/main.ts @@ -39,23 +39,22 @@ if (app.requestSingleInstanceLock()) { app.exit(1); } -function initializeApp(): void { +async function initializeApp(): Promise { let devRelaunchScheduled = false; const settings = getRuntimeSettings(); const logger = initMainLogger(settings); logger.info(`Starting ${app.getName()} version ${app.getVersion()}`); - const appStateFileStorage = createFileStorage({ - filePath: path.join(settings.userDataDir, 'app_state.json'), - debounceWrites: true, + const { + appStateFileStorage, + configFileStorage, + configJsonSchemaFileStorage, + } = await createFileStorages(settings.userDataDir); + + const configService = createConfigService({ + configFile: configFileStorage, + jsonSchemaFile: configJsonSchemaFileStorage, + platform: settings.platform, }); - const appConfigFileStorage = createFileStorage({ - filePath: path.join(settings.userDataDir, 'app_config.json'), - debounceWrites: false, - }); - const configService = createConfigService( - appConfigFileStorage, - settings.platform - ); const windowsManager = new WindowsManager(appStateFileStorage, settings); process.on('uncaughtException', (error, origin) => { @@ -68,7 +67,8 @@ function initializeApp(): void { settings, logger, configService, - fileStorage: appStateFileStorage, + appStateFileStorage, + configFileStorage, windowsManager, }); @@ -92,16 +92,17 @@ function initializeApp(): void { app.on('will-quit', async event => { event.preventDefault(); + const disposeMainProcess = async () => { + try { + await mainProcess.dispose(); + } catch (e) { + logger.error('Failed to gracefully dispose of main process', e); + } + }; - appStateFileStorage.putAllSync(); globalShortcut.unregisterAll(); - try { - await mainProcess.dispose(); - } catch (e) { - logger.error('Failed to gracefully dispose of main process', e); - } finally { - app.exit(); - } + await Promise.all([appStateFileStorage.write(), disposeMainProcess()]); // none of them can throw + app.exit(); }); app.on('quit', () => { @@ -199,3 +200,25 @@ function initMainLogger(settings: types.RuntimeSettings) { return new Logger('Main'); } + +function createFileStorages(userDataDir: string) { + return Promise.all([ + createFileStorage({ + filePath: path.join(userDataDir, 'app_state.json'), + debounceWrites: true, + }), + createFileStorage({ + filePath: path.join(userDataDir, 'app_config.json'), + debounceWrites: false, + discardUpdatesOnLoadError: true, + }), + createFileStorage({ + filePath: path.join(userDataDir, 'schema_app_config.json'), + debounceWrites: false, + }), + ]).then(storages => ({ + appStateFileStorage: storages[0], + configFileStorage: storages[1], + configJsonSchemaFileStorage: storages[2], + })); +} diff --git a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts index ead746ab08572..36be20374ccaa 100644 --- a/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts +++ b/web/packages/teleterm/src/mainProcess/fixtures/mocks.ts @@ -25,10 +25,11 @@ export class MockMainProcessClient implements MainProcessClient { configService: ReturnType; constructor(private runtimeSettings: Partial = {}) { - this.configService = createConfigService( - createMockFileStorage(), - this.getRuntimeSettings().platform - ); + this.configService = createConfigService({ + configFile: createMockFileStorage(), + jsonSchemaFile: createMockFileStorage(), + platform: this.getRuntimeSettings().platform, + }); } getRuntimeSettings(): RuntimeSettings { @@ -63,6 +64,10 @@ export class MockMainProcessClient implements MainProcessClient { async removeTshSymlinkMacOs() { return true; } + + async openConfigFile() { + return ''; + } } const defaultRuntimeSettings = { diff --git a/web/packages/teleterm/src/mainProcess/mainProcess.ts b/web/packages/teleterm/src/mainProcess/mainProcess.ts index 0d9742e42d186..7c370654fcce5 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcess.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcess.ts @@ -50,7 +50,8 @@ type Options = { settings: RuntimeSettings; logger: Logger; configService: ConfigService; - fileStorage: FileStorage; + appStateFileStorage: FileStorage; + configFileStorage: FileStorage; windowsManager: WindowsManager; }; @@ -60,7 +61,8 @@ export default class MainProcess { private readonly configService: ConfigService; private tshdProcess: ChildProcess; private sharedProcess: ChildProcess; - private fileStorage: FileStorage; + private appStateFileStorage: FileStorage; + private configFileStorage: FileStorage; private resolvedChildProcessAddresses: Promise; private windowsManager: WindowsManager; @@ -68,7 +70,8 @@ export default class MainProcess { this.settings = opts.settings; this.logger = opts.logger; this.configService = opts.configService; - this.fileStorage = opts.fileStorage; + this.appStateFileStorage = opts.appStateFileStorage; + this.configFileStorage = opts.configFileStorage; this.windowsManager = opts.windowsManager; } @@ -276,10 +279,16 @@ export default class MainProcess { } }); + ipcMain.handle('main-process-open-config-file', async () => { + const path = this.configFileStorage.getFilePath(); + await shell.openPath(path); + return path; + }); + subscribeToTerminalContextMenuEvent(); subscribeToTabContextMenuEvent(); subscribeToConfigServiceEvents(this.configService); - subscribeToFileStorageEvents(this.fileStorage); + subscribeToFileStorageEvents(this.appStateFileStorage); } private _setAppMenu() { diff --git a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts index 412032abc9cb1..6bb35619a05c6 100644 --- a/web/packages/teleterm/src/mainProcess/mainProcessClient.ts +++ b/web/packages/teleterm/src/mainProcess/mainProcessClient.ts @@ -53,5 +53,8 @@ export default function createMainProcessClient(): MainProcessClient { removeTshSymlinkMacOs() { return ipcRenderer.invoke('main-process-remove-tsh-symlink-macos'); }, + openConfigFile(): Promise { + return ipcRenderer.invoke('main-process-open-config-file'); + }, }; } diff --git a/web/packages/teleterm/src/mainProcess/types.ts b/web/packages/teleterm/src/mainProcess/types.ts index 214706bb049f6..28798a576207b 100644 --- a/web/packages/teleterm/src/mainProcess/types.ts +++ b/web/packages/teleterm/src/mainProcess/types.ts @@ -72,6 +72,9 @@ export type MainProcessClient = { * prompt. The promise gets rejected if osascript encountered an error. */ removeTshSymlinkMacOs(): Promise; + + /** Opens config file and returns a path to it. */ + openConfigFile(): Promise; }; export type ChildProcessAddresses = { @@ -125,11 +128,14 @@ export enum TabContextMenuEventType { export enum ConfigServiceEventType { Get = 'Get', Set = 'Set', - GetStoredConfigErrors = 'GetStoredConfigErrors', + GetConfigError = 'GetConfigError', } export enum FileStorageEventType { Get = 'Get', Put = 'Put', - PutAllSync = 'PutAllSync', + Write = 'Write', + Replace = 'Replace', + GetFilePath = 'GetFilePath', + GetFileLoadingError = 'GetFileLoadingError', } diff --git a/web/packages/teleterm/src/mainProcess/windowsManager.ts b/web/packages/teleterm/src/mainProcess/windowsManager.ts index 81ce00bb106b0..02b2df3e81a4c 100644 --- a/web/packages/teleterm/src/mainProcess/windowsManager.ts +++ b/web/packages/teleterm/src/mainProcess/windowsManager.ts @@ -197,7 +197,7 @@ export class WindowsManager { } private getWindowState(): WindowState { - const windowState = this.fileStorage.get(this.storageKey); + const windowState = this.fileStorage.get(this.storageKey) as WindowState; const getDefaults = () => ({ height: 720, width: 1280, diff --git a/web/packages/teleterm/src/services/config/appConfigSchema.ts b/web/packages/teleterm/src/services/config/appConfigSchema.ts new file mode 100644 index 0000000000000..aa93dbcf6a160 --- /dev/null +++ b/web/packages/teleterm/src/services/config/appConfigSchema.ts @@ -0,0 +1,206 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { z } from 'zod'; + +import { Platform } from 'teleterm/mainProcess/types'; + +import { createKeyboardShortcutSchema } from './keyboardShortcutSchema'; + +export type AppConfigSchema = ReturnType; +export type AppConfig = z.infer; + +export const createAppConfigSchema = (platform: Platform) => { + const defaultKeymap = getDefaultKeymap(platform); + const defaultTerminalFont = getDefaultTerminalFont(platform); + + const shortcutSchema = createKeyboardShortcutSchema(platform); + + // `keymap.` prefix is used in `initUi.ts` in a predicate function. + return z.object({ + 'usageReporting.enabled': z + .boolean() + .default(false) + .describe('Enables collecting of anonymous usage data.'), + 'keymap.tab1': shortcutSchema + .default(defaultKeymap['tab1']) + .describe(getShortcutDesc('open tab 1')), + 'keymap.tab2': shortcutSchema + .default(defaultKeymap['tab2']) + .describe(getShortcutDesc('open tab 2')), + 'keymap.tab3': shortcutSchema + .default(defaultKeymap['tab3']) + .describe(getShortcutDesc('open tab 3')), + 'keymap.tab4': shortcutSchema + .default(defaultKeymap['tab4']) + .describe(getShortcutDesc('open tab 4')), + 'keymap.tab5': shortcutSchema + .default(defaultKeymap['tab5']) + .describe(getShortcutDesc('open tab 5')), + 'keymap.tab6': shortcutSchema + .default(defaultKeymap['tab6']) + .describe(getShortcutDesc('open tab 6')), + 'keymap.tab7': shortcutSchema + .default(defaultKeymap['tab7']) + .describe(getShortcutDesc('open tab 7')), + 'keymap.tab8': shortcutSchema + .default(defaultKeymap['tab8']) + .describe(getShortcutDesc('open tab 8')), + 'keymap.tab9': shortcutSchema + .default(defaultKeymap['tab9']) + .describe(getShortcutDesc('open tab 9')), + 'keymap.closeTab': shortcutSchema + .default(defaultKeymap['closeTab']) + .describe(getShortcutDesc('close a tab')), + 'keymap.newTab': shortcutSchema + .default(defaultKeymap['newTab']) + .describe(getShortcutDesc('open a new tab')), + 'keymap.previousTab': shortcutSchema + .default(defaultKeymap['previousTab']) + .describe(getShortcutDesc('go to the previous tab')), + 'keymap.nextTab': shortcutSchema + .default(defaultKeymap['nextTab']) + .describe(getShortcutDesc('go to the next tab')), + 'keymap.openConnections': shortcutSchema + .default(defaultKeymap['openConnections']) + .describe(getShortcutDesc('open the connection list')), + 'keymap.openClusters': shortcutSchema + .default(defaultKeymap['openClusters']) + .describe(getShortcutDesc('open the cluster selector')), + 'keymap.openProfiles': shortcutSchema + .default(defaultKeymap['openProfiles']) + .describe(getShortcutDesc('open the profile selector')), + 'keymap.openCommandBar': shortcutSchema + .default(defaultKeymap['openCommandBar']) + .describe(getShortcutDesc('open the command bar')), + /** + * This value can be provided by the user and is unsanitized. This means that it cannot be directly interpolated + * in a styled component or used in CSS, as it may inject malicious CSS code. + * Before using it, sanitize it with `CSS.escape` or pass it as a `style` prop. + * Read more https://frontarm.com/james-k-nelson/how-can-i-use-css-in-js-securely/. + */ + 'terminal.fontFamily': z + .string() + .default(defaultTerminalFont) + .describe('Font family for the terminal.'), + 'terminal.fontSize': z + .number() + .int() + .min(1) + .max(256) + .default(15) + .describe('Font size for the terminal.'), + }); +}; + +export type KeyboardShortcutAction = + | 'tab1' + | 'tab2' + | 'tab3' + | 'tab4' + | 'tab5' + | 'tab6' + | 'tab7' + | 'tab8' + | 'tab9' + | 'closeTab' + | 'newTab' + | 'previousTab' + | 'nextTab' + | 'openCommandBar' + | 'openConnections' + | 'openClusters' + | 'openProfiles'; + +const getDefaultKeymap = (platform: Platform) => { + switch (platform) { + case 'win32': + return { + tab1: 'Ctrl+1', + tab2: 'Ctrl+2', + tab3: 'Ctrl+3', + tab4: 'Ctrl+4', + tab5: 'Ctrl+5', + tab6: 'Ctrl+6', + tab7: 'Ctrl+7', + tab8: 'Ctrl+8', + tab9: 'Ctrl+9', + closeTab: 'Ctrl+W', + newTab: 'Ctrl+T', + previousTab: 'Ctrl+Shift+Tab', + nextTab: 'Ctrl+Tab', + openCommandBar: 'Ctrl+K', + openConnections: 'Ctrl+P', + openClusters: 'Ctrl+E', + openProfiles: 'Ctrl+I', + }; + case 'linux': + return { + tab1: 'Alt+1', + tab2: 'Alt+2', + tab3: 'Alt+3', + tab4: 'Alt+4', + tab5: 'Alt+5', + tab6: 'Alt+6', + tab7: 'Alt+7', + tab8: 'Alt+8', + tab9: 'Alt+9', + closeTab: 'Ctrl+W', + newTab: 'Ctrl+T', + previousTab: 'Ctrl+Shift+Tab', + nextTab: 'Ctrl+Tab', + openCommandBar: 'Ctrl+K', + openConnections: 'Ctrl+P', + openClusters: 'Ctrl+E', + openProfiles: 'Ctrl+I', + }; + case 'darwin': + return { + tab1: 'Command+1', + tab2: 'Command+2', + tab3: 'Command+3', + tab4: 'Command+4', + tab5: 'Command+5', + tab6: 'Command+6', + tab7: 'Command+7', + tab8: 'Command+8', + tab9: 'Command+9', + closeTab: 'Command+W', + newTab: 'Command+T', + previousTab: 'Control+Shift+Tab', + nextTab: 'Control+Tab', + openCommandBar: 'Command+K', + openConnections: 'Command+P', + openClusters: 'Command+E', + openProfiles: 'Command+I', + }; + } +}; + +function getDefaultTerminalFont(platform: Platform) { + switch (platform) { + case 'win32': + return 'Consolas, monospace'; + case 'linux': + return "'Droid Sans Mono', monospace"; + case 'darwin': + return 'Menlo, Monaco, monospace'; + } +} + +function getShortcutDesc(actionDesc: string): string { + return `Shortcut to ${actionDesc}. A valid shortcut contains at least one modifier and a single key code, for example "Shift+Tab". Function keys do not require a modifier.`; +} diff --git a/web/packages/teleterm/src/services/config/configService.test.ts b/web/packages/teleterm/src/services/config/configService.test.ts new file mode 100644 index 0000000000000..ab193a3ed1b0d --- /dev/null +++ b/web/packages/teleterm/src/services/config/configService.test.ts @@ -0,0 +1,125 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import Logger, { NullService } from 'teleterm/logger'; +import { createMockFileStorage } from 'teleterm/services/fileStorage/fixtures/mocks'; + +import { createConfigService } from './configService'; + +beforeAll(() => { + Logger.init(new NullService()); +}); + +test('stored and default values are combined', () => { + const configFile = createMockFileStorage(); + configFile.put('usageReporting.enabled', true); + const configService = createConfigService({ + configFile, + jsonSchemaFile: createMockFileStorage(), + platform: 'darwin', + }); + + expect(configService.getConfigError()).toBeUndefined(); + + const usageReportingEnabled = configService.get('usageReporting.enabled'); + expect(usageReportingEnabled.value).toBe(true); + expect(usageReportingEnabled.metadata.isStored).toBe(true); + + const terminalFontSize = configService.get('terminal.fontSize'); + expect(terminalFontSize.value).toBe(15); + expect(terminalFontSize.metadata.isStored).toBe(false); +}); + +test('in case of invalid value a default one is returned', () => { + const configFile = createMockFileStorage(); + configFile.put('usageReporting.enabled', 'abcde'); + const configService = createConfigService({ + configFile: configFile, + jsonSchemaFile: createMockFileStorage(), + platform: 'darwin', + }); + + expect(configService.getConfigError()).toStrictEqual({ + source: 'validation', + errors: [ + { + code: 'invalid_type', + expected: 'boolean', + received: 'string', + message: 'Expected boolean, received string', + path: ['usageReporting.enabled'], + }, + ], + }); + + const usageReportingEnabled = configService.get('usageReporting.enabled'); + expect(usageReportingEnabled.value).toBe(false); + expect(usageReportingEnabled.metadata.isStored).toBe(false); + + const terminalFontSize = configService.get('terminal.fontSize'); + expect(terminalFontSize.value).toBe(15); + expect(terminalFontSize.metadata.isStored).toBe(false); +}); + +test('if config file failed to load correctly the error is returned', () => { + const configFile = createMockFileStorage(); + const error = new Error('Failed to read'); + jest.spyOn(configFile, 'getFileLoadingError').mockReturnValue(error); + + const configService = createConfigService({ + configFile, + jsonSchemaFile: createMockFileStorage(), + platform: 'darwin', + }); + + expect(configService.getConfigError()).toStrictEqual({ + source: 'file-loading', + error, + }); +}); + +test('calling set updated the value in store', () => { + const configFile = createMockFileStorage(); + const configService = createConfigService({ + configFile, + jsonSchemaFile: createMockFileStorage(), + platform: 'darwin', + }); + + configService.set('usageReporting.enabled', true); + + const usageReportingEnabled = configService.get('usageReporting.enabled'); + expect(usageReportingEnabled.value).toBe(true); + expect(usageReportingEnabled.metadata.isStored).toBe(true); +}); + +test('field linking to the json schema and the json schema itself are updated', () => { + const configFile = createMockFileStorage(); + const jsonSchemaFile = createMockFileStorage({ + filePath: '~/config_schema.json', + }); + + jest.spyOn(jsonSchemaFile, 'replace'); + + createConfigService({ + configFile, + jsonSchemaFile, + platform: 'darwin', + }); + + expect(configFile.get('$schema')).toBe('config_schema.json'); + expect(jsonSchemaFile.replace).toHaveBeenCalledTimes(1); +}); diff --git a/web/packages/teleterm/src/services/config/configService.ts b/web/packages/teleterm/src/services/config/configService.ts index 2e153c59939de..ce3d0d278df9a 100644 --- a/web/packages/teleterm/src/services/config/configService.ts +++ b/web/packages/teleterm/src/services/config/configService.ts @@ -14,202 +14,161 @@ * limitations under the License. */ -import { z } from 'zod'; +import path from 'path'; + +import { z, ZodIssue } from 'zod'; +import zodToJsonSchema from 'zod-to-json-schema'; import { FileStorage } from 'teleterm/services/fileStorage'; +import Logger from 'teleterm/logger'; import { Platform } from 'teleterm/mainProcess/types'; -import { createConfigStore } from './configStore'; - -const createAppConfigSchema = (platform: Platform) => { - const defaultKeymap = getDefaultKeymap(platform); - const defaultTerminalFont = getDefaultTerminalFont(platform); - - // Important: all keys except 'usageReporting.enabled' are currently not - // configurable by the user. Before we let the user configure them, - // we need to set up some actual validation, so that for example - // arbitrary CSS cannot be injected into the app through font settings. - // - // However, we want them to be in the config schema, so we included - // them here, but we do not read their value from the stored config. - return z.object({ - 'usageReporting.enabled': z.boolean().default(false), - 'keymap.tab1': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-1']) - ), - 'keymap.tab2': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-2']) - ), - 'keymap.tab3': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-3']) - ), - 'keymap.tab4': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-4']) - ), - 'keymap.tab5': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-5']) - ), - 'keymap.tab6': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-6']) - ), - 'keymap.tab7': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-7']) - ), - 'keymap.tab8': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-8']) - ), - 'keymap.tab9': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-9']) - ), - 'keymap.tabClose': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-close']) - ), - 'keymap.tabNew': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-new']) - ), - 'keymap.tabPrevious': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-previous']) - ), - 'keymap.tabNext': omitStoredConfigValue( - z.string().default(defaultKeymap['tab-next']) - ), - 'keymap.toggleConnections': omitStoredConfigValue( - z.string().default(defaultKeymap['toggle-connections']) - ), - 'keymap.toggleClusters': omitStoredConfigValue( - z.string().default(defaultKeymap['toggle-clusters']) - ), - 'keymap.toggleIdentity': omitStoredConfigValue( - z.string().default(defaultKeymap['toggle-identity']) - ), - 'keymap.openQuickInput': omitStoredConfigValue( - z.string().default(defaultKeymap['open-quick-input']) - ), - /** - * This value can be provided by the user and is unsanitized. This means that it cannot be directly interpolated - * in a styled component or used in CSS, as it may inject malicious CSS code. - * Before using it, sanitize it with `CSS.escape` or pass it as a `style` prop. - * Read more https://frontarm.com/james-k-nelson/how-can-i-use-css-in-js-securely/. - */ - 'terminal.fontFamily': z.string().default(defaultTerminalFont), - 'terminal.fontSize': z.number().int().min(1).max(256).default(15), - }); -}; +import { + createAppConfigSchema, + AppConfigSchema, + AppConfig, +} from './appConfigSchema'; -const omitStoredConfigValue = (schema: z.ZodType) => - z.preprocess(() => undefined, schema); +const logger = new Logger('ConfigService'); -export type AppConfig = z.infer>; +type FileLoadingError = { + source: 'file-loading'; + error: Error; +}; -/** - * Modifier keys must be defined in the following order: - * Command-Control-Option-Shift for macOS - * Ctrl-Alt-Shift for other platforms - */ -export type KeyboardShortcutType = - | 'tab-1' - | 'tab-2' - | 'tab-3' - | 'tab-4' - | 'tab-5' - | 'tab-6' - | 'tab-7' - | 'tab-8' - | 'tab-9' - | 'tab-close' - | 'tab-new' - | 'tab-previous' - | 'tab-next' - | 'open-quick-input' - | 'toggle-connections' - | 'toggle-clusters' - | 'toggle-identity'; - -export type KeyboardShortcutsConfig = Record; -const getDefaultKeymap = (platform: Platform) => { - switch (platform) { - case 'win32': - return { - 'tab-1': 'Ctrl-1', - 'tab-2': 'Ctrl-2', - 'tab-3': 'Ctrl-3', - 'tab-4': 'Ctrl-4', - 'tab-5': 'Ctrl-5', - 'tab-6': 'Ctrl-6', - 'tab-7': 'Ctrl-7', - 'tab-8': 'Ctrl-8', - 'tab-9': 'Ctrl-9', - 'tab-close': 'Ctrl-W', - 'tab-new': 'Ctrl-T', - 'tab-previous': 'Ctrl-Shift-Tab', - 'tab-next': 'Ctrl-Tab', - 'open-quick-input': 'Ctrl-K', - 'toggle-connections': 'Ctrl-P', - 'toggle-clusters': 'Ctrl-E', - 'toggle-identity': 'Ctrl-I', - }; - case 'linux': - return { - 'tab-1': 'Alt-1', - 'tab-2': 'Alt-2', - 'tab-3': 'Alt-3', - 'tab-4': 'Alt-4', - 'tab-5': 'Alt-5', - 'tab-6': 'Alt-6', - 'tab-7': 'Alt-7', - 'tab-8': 'Alt-8', - 'tab-9': 'Alt-9', - 'tab-close': 'Ctrl-W', - 'tab-new': 'Ctrl-T', - 'tab-previous': 'Ctrl-Shift-Tab', - 'tab-next': 'Ctrl-Tab', - 'open-quick-input': 'Ctrl-K', - 'toggle-connections': 'Ctrl-P', - 'toggle-clusters': 'Ctrl-E', - 'toggle-identity': 'Ctrl-I', - }; - case 'darwin': - return { - 'tab-1': 'Command-1', - 'tab-2': 'Command-2', - 'tab-3': 'Command-3', - 'tab-4': 'Command-4', - 'tab-5': 'Command-5', - 'tab-6': 'Command-6', - 'tab-7': 'Command-7', - 'tab-8': 'Command-8', - 'tab-9': 'Command-9', - 'tab-close': 'Command-W', - 'tab-new': 'Command-T', - 'tab-previous': 'Control-Shift-Tab', - 'tab-next': 'Control-Tab', - 'open-quick-input': 'Command-K', - 'toggle-connections': 'Command-P', - 'toggle-clusters': 'Command-E', - 'toggle-identity': 'Command-I', - }; - } +type ValidationError = { + source: 'validation'; + errors: ZodIssue[]; }; -function getDefaultTerminalFont(platform: Platform) { - switch (platform) { - case 'win32': - return "'Consolas', 'Courier New', monospace"; - case 'linux': - return "'Droid Sans Mono', 'Courier New', monospace, 'Droid Sans Fallback'"; - case 'darwin': - return "Menlo, Monaco, 'Courier New', monospace"; - } +type ConfigError = FileLoadingError | ValidationError; + +export interface ConfigService { + get( + key: K + ): { value: AppConfig[K]; metadata: { isStored: boolean } }; + + set(key: K, value: AppConfig[K]): void; + + /** + * Returns validation errors or an error that occurred during loading the config file (this means IO and syntax errors). + * This error has to be checked during the initialization of the app. + * + * The reason we have a getter for this error instead of making `createConfigService` fail with an error + * is that in the presence of this error we want to notify about it and then continue with default values: + * - If validation errors occur, the incorrect values are replaced with the defaults. + * - In case of an error coming from loading the file, all values are replaced with the defaults. + * */ + getConfigError(): ConfigError | undefined; } -export function createConfigService( - appConfigFileStorage: FileStorage, - platform: Platform -) { - return createConfigStore( - createAppConfigSchema(platform), - appConfigFileStorage +export function createConfigService({ + configFile, + jsonSchemaFile, + platform, +}: { + configFile: FileStorage; + jsonSchemaFile: FileStorage; + platform: Platform; +}): ConfigService { + const schema = createAppConfigSchema(platform); + updateJsonSchema({ schema, configFile, jsonSchemaFile }); + + const { + storedConfig, + configWithDefaults, + errors: validationErrors, + } = validateStoredConfig(schema, configFile); + + return { + get: key => ({ + value: configWithDefaults[key], + metadata: { isStored: storedConfig[key] !== undefined }, + }), + set: (key, value) => { + configFile.put(key as string, value); + configWithDefaults[key] = value; + storedConfig[key] = value; + }, + getConfigError: () => { + const fileLoadingError = configFile.getFileLoadingError(); + if (fileLoadingError) { + return { + source: 'file-loading', + error: fileLoadingError, + }; + } + if (validationErrors) { + return { + source: 'validation', + errors: validationErrors, + }; + } + }, + }; +} + +function updateJsonSchema({ + schema, + configFile, + jsonSchemaFile, +}: { + schema: AppConfigSchema; + configFile: FileStorage; + jsonSchemaFile: FileStorage; +}): void { + const jsonSchema = zodToJsonSchema( + // Add $schema field to prevent marking it as a not allowed property. + schema.extend({ $schema: z.string() }), + { $refStrategy: 'none' } ); + const jsonSchemaFileName = path.basename(jsonSchemaFile.getFilePath()); + const jsonSchemaFileNameInConfig = configFile.get('$schema'); + + jsonSchemaFile.replace(jsonSchema); + + if (jsonSchemaFileNameInConfig !== jsonSchemaFileName) { + configFile.put('$schema', jsonSchemaFileName); + } } -export type ConfigService = ReturnType; +function validateStoredConfig( + schema: AppConfigSchema, + configFile: FileStorage +): { + storedConfig: Partial; + configWithDefaults: AppConfig; + errors: ZodIssue[] | undefined; +} { + const parse = (data: Partial) => schema.safeParse(data); + + const storedConfig = configFile.get() as Partial; + const parsed = parse(storedConfig); + if (parsed.success === true) { + return { + storedConfig, + configWithDefaults: parsed.data, + errors: undefined, + }; + } + const withoutInvalidKeys = { ...storedConfig }; + parsed.error.issues.forEach(error => { + // remove only top-level keys + delete withoutInvalidKeys[error.path[0]]; + logger.info( + `Invalid config key, error: ${error.message} at ${error.path.join('.')}` + ); + }); + const reParsed = parse(withoutInvalidKeys); + if (reParsed.success === false) { + // it can happen when a default value does not pass validation + throw new Error( + `Re-parsing config file failed \n${reParsed.error.message}` + ); + } + return { + storedConfig: withoutInvalidKeys, + configWithDefaults: reParsed.data, + errors: parsed.error.issues, + }; +} diff --git a/web/packages/teleterm/src/services/config/configServiceClient.ts b/web/packages/teleterm/src/services/config/configServiceClient.ts index dd761a8c6de10..e12389c89585c 100644 --- a/web/packages/teleterm/src/services/config/configServiceClient.ts +++ b/web/packages/teleterm/src/services/config/configServiceClient.ts @@ -33,8 +33,8 @@ export function subscribeToConfigServiceEvents( return (event.returnValue = configService.get(item.path)); case ConfigServiceEventType.Set: return configService.set(item.path, item.value); - case ConfigServiceEventType.GetStoredConfigErrors: - return (event.returnValue = configService.getStoredConfigErrors()); + case ConfigServiceEventType.GetConfigError: + return (event.returnValue = configService.getConfigError()); } } ); @@ -54,10 +54,10 @@ export function createConfigServiceClient(): ConfigService { value, }); }, - getStoredConfigErrors: () => { + getConfigError: () => { return ipcRenderer.sendSync( ConfigServiceEventChannel, - ConfigServiceEventType.GetStoredConfigErrors + ConfigServiceEventType.GetConfigError ); }, }; diff --git a/web/packages/teleterm/src/services/config/configStore.test.ts b/web/packages/teleterm/src/services/config/configStore.test.ts deleted file mode 100644 index fd892edf947a2..0000000000000 --- a/web/packages/teleterm/src/services/config/configStore.test.ts +++ /dev/null @@ -1,82 +0,0 @@ -/** - * Copyright 2023 Gravitational, Inc - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { z } from 'zod'; - -import Logger, { NullService } from 'teleterm/logger'; -import { createMockFileStorage } from 'teleterm/services/fileStorage/fixtures/mocks'; - -import { createConfigStore } from './configStore'; - -beforeAll(() => { - Logger.init(new NullService()); -}); - -const schema = z.object({ - 'fonts.monoFamily': z.string().default('Arial'), - 'usageReporting.enabled': z.boolean().default(false), -}); - -test('stored and default values are combined', () => { - const fileStorage = createMockFileStorage(); - fileStorage.put('usageReporting.enabled', true); - const configStore = createConfigStore(schema, fileStorage); - - expect(configStore.getStoredConfigErrors()).toBeUndefined(); - - const usageReportingEnabled = configStore.get('usageReporting.enabled'); - expect(usageReportingEnabled.value).toBe(true); - expect(usageReportingEnabled.metadata.isStored).toBe(true); - - const monoFontFamily = configStore.get('fonts.monoFamily'); - expect(monoFontFamily.value).toBe('Arial'); - expect(monoFontFamily.metadata.isStored).toBe(false); -}); - -test('in case of invalid value a default one is returned', () => { - const fileStorage = createMockFileStorage(); - fileStorage.put('usageReporting.enabled', 'abcde'); - const configStore = createConfigStore(schema, fileStorage); - - expect(configStore.getStoredConfigErrors()).toStrictEqual([ - { - code: 'invalid_type', - expected: 'boolean', - received: 'string', - message: 'Expected boolean, received string', - path: ['usageReporting.enabled'], - }, - ]); - - const usageReportingEnabled = configStore.get('usageReporting.enabled'); - expect(usageReportingEnabled.value).toBe(false); - expect(usageReportingEnabled.metadata.isStored).toBe(false); - - const monoFontFamily = configStore.get('fonts.monoFamily'); - expect(monoFontFamily.value).toBe('Arial'); - expect(monoFontFamily.metadata.isStored).toBe(false); -}); - -test('calling set updated the value in store', () => { - const fileStorage = createMockFileStorage(); - const configStore = createConfigStore(schema, fileStorage); - - configStore.set('usageReporting.enabled', true); - - const usageReportingEnabled = configStore.get('usageReporting.enabled'); - expect(usageReportingEnabled.value).toBe(true); - expect(usageReportingEnabled.metadata.isStored).toBe(true); -}); diff --git a/web/packages/teleterm/src/services/config/configStore.ts b/web/packages/teleterm/src/services/config/configStore.ts deleted file mode 100644 index 3c754d7775ef1..0000000000000 --- a/web/packages/teleterm/src/services/config/configStore.ts +++ /dev/null @@ -1,90 +0,0 @@ -/** - * Copyright 2023 Gravitational, Inc - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -import { z, ZodIssue } from 'zod'; - -import { FileStorage } from 'teleterm/services/fileStorage'; -import Logger from 'teleterm/logger'; - -const logger = new Logger('ConfigStore'); - -export function createConfigStore< - Schema extends z.ZodTypeAny, - Shape = z.infer ->(schema: Schema, fileStorage: FileStorage) { - const { storedConfig, configWithDefaults, errors } = validateStoredConfig(); - - function get( - key: K - ): { value: Shape[K]; metadata: { isStored: boolean } } { - return { - value: configWithDefaults[key], - metadata: { isStored: storedConfig[key] !== undefined }, - }; - } - - function set(key: K, value: Shape[K]): void { - fileStorage.put(key as string, value); - configWithDefaults[key] = value; - storedConfig[key] = value; - } - - function getStoredConfigErrors(): ZodIssue[] | undefined { - return errors; - } - - function parse(data: Partial) { - return schema.safeParse(data); - } - - //TODO (gzdunek): syntax errors of the JSON file are silently ignored, report - // them to the user too - function validateStoredConfig(): { - storedConfig: Partial; - configWithDefaults: Shape; - errors: ZodIssue[] | undefined; - } { - const storedConfig = fileStorage.get>(); - const parsed = parse(storedConfig); - if (parsed.success === true) { - return { - storedConfig, - configWithDefaults: parsed.data, - errors: undefined, - }; - } - const withoutInvalidKeys = { ...storedConfig }; - parsed.error.issues.forEach(error => { - // remove only top-level keys - delete withoutInvalidKeys[error.path[0]]; - logger.info( - `Invalid config key, error: ${error.message} at ${error.path.join('.')}` - ); - }); - const reParsed = parse(withoutInvalidKeys); - if (reParsed.success === false) { - // it should not occur after removing invalid keys, but just in case - throw new Error('Re-parsing config file failed', reParsed.error.cause); - } - return { - storedConfig: withoutInvalidKeys, - configWithDefaults: reParsed.data, - errors: parsed.error.issues, - }; - } - - return { get, set, getStoredConfigErrors }; -} diff --git a/web/packages/teleterm/src/services/config/fixtures/mocks.ts b/web/packages/teleterm/src/services/config/fixtures/mocks.ts index fd094e97ebaf3..d500814bdd8db 100644 --- a/web/packages/teleterm/src/services/config/fixtures/mocks.ts +++ b/web/packages/teleterm/src/services/config/fixtures/mocks.ts @@ -27,8 +27,8 @@ export function createMockConfigService( set(key, value) { values[key] = value; }, - getStoredConfigErrors() { - return []; + getConfigError() { + return undefined; }, }; } diff --git a/web/packages/teleterm/src/services/config/index.ts b/web/packages/teleterm/src/services/config/index.ts index 8bc8376c8e463..9fd174ba63317 100644 --- a/web/packages/teleterm/src/services/config/index.ts +++ b/web/packages/teleterm/src/services/config/index.ts @@ -16,3 +16,4 @@ export * from './configService'; export * from './configServiceClient'; +export { AppConfig, KeyboardShortcutAction } from './appConfigSchema'; diff --git a/web/packages/teleterm/src/services/config/keyboardShortcutSchema.test.ts b/web/packages/teleterm/src/services/config/keyboardShortcutSchema.test.ts new file mode 100644 index 0000000000000..f2f08aef049f6 --- /dev/null +++ b/web/packages/teleterm/src/services/config/keyboardShortcutSchema.test.ts @@ -0,0 +1,107 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { z, ZodError } from 'zod'; + +import { + createKeyboardShortcutSchema, + invalidModifierIssue, + invalidKeyCodeIssue, + duplicateModifierIssue, + missingModifierIssue, +} from './keyboardShortcutSchema'; + +const schema = z.object({ + 'keymap.tab1': createKeyboardShortcutSchema('darwin'), +}); + +function getZodError(...issues: any[]): z.ZodError { + return new ZodError( + issues.map(issue => ({ + ...issue, + path: ['keymap.tab1'], + })) + ); +} + +test('multi-parts accelerator is parsed correctly', () => { + const parsed = schema.parse({ 'keymap.tab1': 'Command+Shift+1' }); + expect(parsed).toStrictEqual({ 'keymap.tab1': 'Shift+Command+1' }); +}); + +test('single-part accelerator is allowed for function keys', () => { + const parsed = schema.parse({ 'keymap.tab1': 'F1' }); + expect(parsed).toStrictEqual({ 'keymap.tab1': 'F1' }); +}); + +test('single-part accelerator is not allowed for non-function keys', () => { + const parse = () => schema.parse({ 'keymap.tab1': '1' }); + expect(parse).toThrow(getZodError(missingModifierIssue('1'))); +}); + +test('accelerator parts are sorted in the correct order', () => { + const parsed = schema.parse({ 'keymap.tab1': 'Shift+1+Command' }); + expect(parsed).toStrictEqual({ 'keymap.tab1': 'Shift+Command+1' }); +}); + +test('accelerator with whitespaces is parsed correctly', () => { + const parsed = schema.parse({ 'keymap.tab1': ' Shift + 1 + Command ' }); + expect(parsed).toStrictEqual({ 'keymap.tab1': 'Shift+Command+1' }); +}); + +test('empty accelerator is allowed', () => { + const parsed = schema.parse({ 'keymap.tab1': '' }); + expect(parsed).toStrictEqual({ 'keymap.tab1': '' }); +}); + +test('lowercase single characters are allowed and converted to uppercase', () => { + const parsed = schema.parse({ 'keymap.tab1': 'Shift+Command+a' }); + expect(parsed).toStrictEqual({ 'keymap.tab1': 'Shift+Command+A' }); +}); + +test('parsing fails when incorrect physical key is passed', () => { + const parse = () => schema.parse({ 'keymap.tab1': 'Shift+12' }); + expect(parse).toThrow(getZodError(invalidKeyCodeIssue('12'))); +}); + +test('parsing fails when multiple key codes are passed', () => { + const parse = () => schema.parse({ 'keymap.tab1': 'Shift+Space+Tab' }); + expect(parse).toThrow( + getZodError( + invalidModifierIssue(['Space'], ['Control', 'Option', 'Shift', 'Command']) + ) + ); +}); + +test('parsing fails when only modifiers are passed', () => { + const parse = () => schema.parse({ 'keymap.tab1': 'Command+Shift' }); + expect(parse).toThrow(getZodError(invalidKeyCodeIssue('Command'))); +}); + +test('parsing fails when duplicate invalid modifiers are passed', () => { + const parse = () => schema.parse({ 'keymap.tab1': 'Comm+Comm+1' }); + expect(parse).toThrow( + getZodError( + duplicateModifierIssue(), + invalidModifierIssue(['Comm'], ['Control', 'Option', 'Shift', 'Command']) + ) + ); +}); + +test('parsing fails when duplicate valid modifiers are passed', () => { + const parse = () => schema.parse({ 'keymap.tab1': 'Command+I+Command' }); + expect(parse).toThrow(getZodError(duplicateModifierIssue())); +}); diff --git a/web/packages/teleterm/src/services/config/keyboardShortcutSchema.ts b/web/packages/teleterm/src/services/config/keyboardShortcutSchema.ts new file mode 100644 index 0000000000000..fda66e062ddcb --- /dev/null +++ b/web/packages/teleterm/src/services/config/keyboardShortcutSchema.ts @@ -0,0 +1,173 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { z } from 'zod'; + +import { Platform } from 'teleterm/mainProcess/types'; + +export function invalidKeyCodeIssue(wrongKeyCode: string): z.IssueData { + return { + code: z.ZodIssueCode.custom, + message: `"${wrongKeyCode}" cannot be used as a key code.`, + }; +} + +export function invalidModifierIssue( + wrongModifiers: string[], + validModifiers: string[] +): z.IssueData { + const formatList = (items: string[]) => + `${items.map(m => `"${m}"`).join(', ')}`; + return { + code: z.ZodIssueCode.custom, + message: `${formatList( + wrongModifiers + )} cannot be used as a modifier. Valid modifiers are: ${formatList( + validModifiers + )}.`, + }; +} + +export function duplicateModifierIssue(): z.IssueData { + return { + code: z.ZodIssueCode.custom, + message: `Duplicate modifier found.`, + }; +} + +export function missingModifierIssue(keyCode: string): z.IssueData { + return { + code: z.ZodIssueCode.custom, + message: `"${keyCode}" must be used together with a modifier.`, + }; +} + +export function createKeyboardShortcutSchema(platform: Platform) { + const allowedModifiers = getSupportedModifiers(platform); + + return z + .string() + .transform(s => s.trim().split(/\s?\+\s?/)) + .transform(putModifiersFirst(allowedModifiers)) + .superRefine(validateKeyCodeAndModifiers(allowedModifiers)) + .transform(adjustCasing) + .transform(s => s.join('+')); +} + +function putModifiersFirst( + allowedModifiers: string[] +): (tokens: string[]) => string[] { + return tokens => + tokens.sort((a, b) => { + if (allowedModifiers.indexOf(a) === -1) { + return 1; + } + if (allowedModifiers.indexOf(b) === -1) { + return -1; + } + return allowedModifiers.indexOf(a) - allowedModifiers.indexOf(b); + }); +} + +/** Currently, works only for single characters. + * An accelerator string is compared to a string generated from event properties + * (so it should be case-insensitive too). + * But what is more important, this string is used also for tooltips. + * If we allow "CTRL+SHIFT+PAGEUP" then we cannot display it as "Ctrl+Shift+PageUp". + */ +function adjustCasing(tokens: string[]): string[] { + return tokens.map(token => { + if (token.length === 1) { + return token.toUpperCase(); + } + return token; + }); +} + +function validateKeyCodeAndModifiers( + allowedModifiers: string[] +): (tokens: string[], ctx: z.RefinementCtx) => void { + return (tokens, ctx) => { + // empty accelerator disables the shortcut + if (tokens.join('') === '') { + return z.NEVER; + } + + const [keyCode, ...modifiers] = [...tokens].reverse(); + + const keyCodeUppercase = + keyCode.length === 1 // letters + ? keyCode.toUpperCase() + : keyCode; + if (!ALLOWED_KEY_CODES.includes(keyCodeUppercase)) { + ctx.addIssue(invalidKeyCodeIssue(keyCode)); + } + + if (modifiers.length !== new Set(modifiers).size) { + ctx.addIssue(duplicateModifierIssue()); + } + + const invalidModifiers = modifiers.filter( + modifier => !allowedModifiers.includes(modifier) + ); + if (invalidModifiers.length) { + ctx.addIssue( + invalidModifierIssue( + Array.from(new Set(invalidModifiers)), + allowedModifiers + ) + ); + } + + if (!FUNCTION_KEYS.includes(keyCode) && !modifiers.length) { + ctx.addIssue(missingModifierIssue(keyCode)); + } + }; +} + +/** Returns allowed modifiers for a given platform in the correct order. + * https://developer.apple.com/design/human-interface-guidelines/inputs/keyboards/#custom-keyboard-shortcuts + */ +function getSupportedModifiers(platform: Platform): string[] { + switch (platform) { + case 'win32': + case 'linux': + return ['Ctrl', 'Alt', 'Shift']; + case 'darwin': + return ['Control', 'Option', 'Shift', 'Command']; + } +} + +function generateRange(start: number, end: number): string[] { + return new Array(end - start + 1) + .fill(undefined) + .map((_, i) => (i + start).toString()); +} + +const FUNCTION_KEYS = generateRange(1, 24).map(key => `F${key}`); + +// subset of https://github.com/electron/electron/blob/49df19214ea3abaf0ad91adf3d374cba32abd521/docs/api/accelerator.md#available-key-codes +// prettier-ignore +const ALLOWED_KEY_CODES = [ + ...generateRange(0, 9), // 0-9 range + ...FUNCTION_KEYS, + 'A', 'B', 'C', 'D', 'E', 'F', 'G', 'H', 'I', 'J', 'K', 'L', 'M', 'N', 'O', 'P', 'Q', 'R', 'S', + 'T', 'U', 'V', 'W', 'X', 'Y', 'Z', + 'Space', 'Tab', 'CapsLock', 'NumLock', 'ScrollLock', 'Backspace', 'Delete', 'Insert', 'Enter', + 'Up', 'Down', 'Left', 'Right', + 'Home', 'End', 'PageUp', 'PageDown', 'Escape', 'IntlBackslash', + ',', '.', '/', '\\', '`', '-', '=', ';', "'", '[', ']', +]; diff --git a/web/packages/teleterm/src/services/fileStorage/fileStorage.ts b/web/packages/teleterm/src/services/fileStorage/fileStorage.ts index 83e72dd57a713..03da0187b424c 100644 --- a/web/packages/teleterm/src/services/fileStorage/fileStorage.ts +++ b/web/packages/teleterm/src/services/fileStorage/fileStorage.ts @@ -14,7 +14,7 @@ * limitations under the License. */ -import fs, { existsSync, readFileSync, writeFileSync } from 'fs'; +import fs from 'fs/promises'; import { debounce } from 'shared/utils/highbar'; @@ -23,63 +23,119 @@ import Logger from 'teleterm/logger'; const logger = new Logger('FileStorage'); export interface FileStorage { - put(path: string, json: any): void; + /** Asynchronously updates value for a given key. */ + put(key: string, json: any): void; - putAllSync(): void; + /** Asynchronously replaces the entire storage state with a new value. */ + replace(json: any): void; - get(path?: string): T; + /** Asynchronously writes the storage state to disk. */ + write(): Promise; + + /** Returns value for a given key. If the key is omitted, the entire storage state is returned. */ + // TODO(ravicious): Add a generic type to createFileStorage rather than returning `unknown`. + // https://github.com/gravitational/teleport/pull/22728#discussion_r1129566566 + get(key?: string): unknown; + + /** Returns the file path used to create the storage. */ + getFilePath(): string; + + /** Returns the error that could occur while reading and parsing the file. */ + getFileLoadingError(): Error | undefined; } -export function createFileStorage(opts: { +export async function createFileStorage(opts: { filePath: string; debounceWrites: boolean; -}): FileStorage { + /** Prevents state updates when the file has not been loaded correctly, so its content will not be overwritten. */ + discardUpdatesOnLoadError?: boolean; +}): Promise { if (!opts || !opts.filePath) { throw Error('missing filePath'); } const { filePath } = opts; - const state = loadState(opts.filePath); - function put(key: string, json: any) { - state[key] = json; - const text = stringify(state); + let state: any, error: Error | undefined; + try { + state = await loadState(filePath); + } catch (e) { + state = {}; + error = e; + logger.error(`Cannot read ${filePath} file`, e); + } - opts.debounceWrites - ? writeFileDebounced(filePath, text) - : writeFile(filePath, text); + const discardUpdates = error && opts.discardUpdatesOnLoadError; + + function put(key: string, json: any): void { + if (discardUpdates) { + return; + } + state[key] = json; + stringifyAndWrite(); } - function putAllSync() { + function write(): Promise { + if (discardUpdates) { + return; + } const text = stringify(state); - try { - fs.writeFileSync(filePath, text); - } catch (error) { - logger.error(`Cannot update ${filePath} file`, error); + writeFile(filePath, text); + } + + function replace(json: any): void { + if (discardUpdates) { + return; } + state = json; + stringifyAndWrite(); } - function get(key?: string): T { - return key ? state[key] : (state as T); + function get(key?: string): unknown { + return key ? state[key] : state; + } + + function getFilePath(): string { + return opts.filePath; + } + + function getFileLoadingError(): Error | undefined { + return error; + } + + function stringifyAndWrite(): void { + const text = stringify(state); + + opts.debounceWrites + ? writeFileDebounced(filePath, text) + : writeFile(filePath, text); } return { put, - putAllSync, + write, get, + replace, + getFilePath, + getFileLoadingError, }; } -function loadState(filePath: string) { - try { - if (!existsSync(filePath)) { - writeFileSync(filePath, '{}'); - } +async function loadState(filePath: string): Promise { + const file = await readOrCreateFile(filePath); + return JSON.parse(file); +} - return JSON.parse(readFileSync(filePath, { encoding: 'utf-8' })); +async function readOrCreateFile(filePath: string): Promise { + try { + return await fs.readFile(filePath, { encoding: 'utf-8' }); } catch (error) { - logger.error(`Cannot read ${filePath} file`, error); - return {}; + const defaultValue = '{}'; + if (error?.code === 'ENOENT') { + await fs.writeFile(filePath, defaultValue); + return defaultValue; + } + throw error; } } @@ -93,6 +149,6 @@ const writeFileDebounced = debounce( ); const writeFile = (filePath: string, text: string) => - fs.promises.writeFile(filePath, text).catch(error => { + fs.writeFile(filePath, text).catch(error => { logger.error(`Cannot update ${filePath} file`, error); }); diff --git a/web/packages/teleterm/src/services/fileStorage/fileStorageClient.ts b/web/packages/teleterm/src/services/fileStorage/fileStorageClient.ts index c9ffff816b688..defcb951b13bf 100644 --- a/web/packages/teleterm/src/services/fileStorage/fileStorageClient.ts +++ b/web/packages/teleterm/src/services/fileStorage/fileStorageClient.ts @@ -29,11 +29,17 @@ export function subscribeToFileStorageEvents(configService: FileStorage): void { (event, eventType: FileStorageEventType, item) => { switch (eventType) { case FileStorageEventType.Get: - return (event.returnValue = configService.get(item.path)); + return (event.returnValue = configService.get(item.key)); case FileStorageEventType.Put: - return configService.put(item.path, item.json); - case FileStorageEventType.PutAllSync: - return configService.putAllSync(); + return configService.put(item.key, item.json); + case FileStorageEventType.Write: + return configService.write(); + case FileStorageEventType.Replace: + return configService.replace(item.json); + case FileStorageEventType.GetFilePath: + return configService.getFilePath(); + case FileStorageEventType.GetFileLoadingError: + return configService.getFileLoadingError(); } } ); @@ -41,19 +47,35 @@ export function subscribeToFileStorageEvents(configService: FileStorage): void { export function createFileStorageClient(): FileStorage { return { - get: path => + get: key => ipcRenderer.sendSync(FileStorageEventChannel, FileStorageEventType.Get, { - path, + key, }), - put: (path, json) => + put: (key, json) => ipcRenderer.send(FileStorageEventChannel, FileStorageEventType.Put, { - path, + key, json, }), - putAllSync: () => - ipcRenderer.send( + write: () => + ipcRenderer.invoke( FileStorageEventChannel, - FileStorageEventType.PutAllSync, + FileStorageEventType.Write, + {} + ), + replace: json => + ipcRenderer.send(FileStorageEventChannel, FileStorageEventType.Replace, { + json, + }), + getFilePath: () => + ipcRenderer.sendSync( + FileStorageEventChannel, + FileStorageEventType.GetFilePath, + {} + ), + getFileLoadingError: () => + ipcRenderer.sendSync( + FileStorageEventChannel, + FileStorageEventType.GetFileLoadingError, {} ), }; diff --git a/web/packages/teleterm/src/services/fileStorage/fixtures/mocks.ts b/web/packages/teleterm/src/services/fileStorage/fixtures/mocks.ts index 5fce1077ffe17..a7243e06836a3 100644 --- a/web/packages/teleterm/src/services/fileStorage/fixtures/mocks.ts +++ b/web/packages/teleterm/src/services/fileStorage/fixtures/mocks.ts @@ -16,17 +16,31 @@ import { FileStorage } from 'teleterm/services/fileStorage'; -export function createMockFileStorage(): FileStorage { +export function createMockFileStorage(opts?: { + filePath: string; +}): FileStorage { let state = {}; return { - put(path: string, json: any) { - state[path] = json; + put(key: string, json: any) { + state[key] = json; }, get(key?: string): T { return key ? state[key] : (state as T); }, - putAllSync() {}, + async write() {}, + + replace(json: any) { + state = json; + }, + + getFilePath() { + return opts?.filePath || ''; + }, + + getFileLoadingError() { + return undefined; + }, }; } diff --git a/web/packages/teleterm/src/ui/Documents/KeyboardShortcutsPanel.tsx b/web/packages/teleterm/src/ui/Documents/KeyboardShortcutsPanel.tsx index c62150a70bbab..89643c732c28f 100644 --- a/web/packages/teleterm/src/ui/Documents/KeyboardShortcutsPanel.tsx +++ b/web/packages/teleterm/src/ui/Documents/KeyboardShortcutsPanel.tsx @@ -21,31 +21,31 @@ import styled from 'styled-components'; import Document from 'teleterm/ui/Document'; import { useKeyboardShortcutFormatters } from 'teleterm/ui/services/keyboardShortcuts'; -import { KeyboardShortcutType } from 'teleterm/services/config'; +import { KeyboardShortcutAction } from 'teleterm/services/config'; export function KeyboardShortcutsPanel() { - const { getShortcut } = useKeyboardShortcutFormatters(); + const { getAccelerator } = useKeyboardShortcutFormatters(); - const items: { title: string; shortcutKey: KeyboardShortcutType }[] = [ + const items: { title: string; shortcutAction: KeyboardShortcutAction }[] = [ { title: 'Open New Tab', - shortcutKey: 'tab-new', + shortcutAction: 'newTab', }, { title: 'Go To Next Tab', - shortcutKey: 'tab-next', + shortcutAction: 'nextTab', }, { title: 'Open Connections', - shortcutKey: 'toggle-connections', + shortcutAction: 'openConnections', }, { title: 'Open Clusters', - shortcutKey: 'toggle-clusters', + shortcutAction: 'openClusters', }, { title: 'Open Profiles', - shortcutKey: 'toggle-identity', + shortcutAction: 'openProfiles', }, ]; @@ -55,10 +55,10 @@ export function KeyboardShortcutsPanel() { {items.map(item => ( ))} @@ -66,14 +66,14 @@ export function KeyboardShortcutsPanel() { ); } -function Entry(props: { title: string; shortcut: string }) { +function Entry(props: { title: string; accelerator: string }) { return ( <> {props.title} - {props.shortcut} + {props.accelerator} ); diff --git a/web/packages/teleterm/src/ui/QuickInput/useQuickInput.ts b/web/packages/teleterm/src/ui/QuickInput/useQuickInput.ts index b465f8dcb0e65..621ae556039bc 100644 --- a/web/packages/teleterm/src/ui/QuickInput/useQuickInput.ts +++ b/web/packages/teleterm/src/ui/QuickInput/useQuickInput.ts @@ -29,7 +29,7 @@ import { Suggestion, } from 'teleterm/ui/services/quickInput/types'; import { routing } from 'teleterm/ui/uri'; -import { KeyboardShortcutType } from 'teleterm/services/config'; +import { KeyboardShortcutAction } from 'teleterm/services/config'; import { assertUnreachable, retryWithRelogin } from '../utils'; @@ -67,14 +67,15 @@ export default function useQuickInput() { }); } } + get(); }, [parseResult]); const hasSuggestions = suggestionsAttempt.status === 'success' && suggestionsAttempt.data.length > 0; - const openQuickInputShortcutKey: KeyboardShortcutType = 'open-quick-input'; - const { getShortcut } = useKeyboardShortcutFormatters(); + const openCommandBarShortcutAction: KeyboardShortcutAction = 'openCommandBar'; + const { getAccelerator } = useKeyboardShortcutFormatters(); const onFocus = (e: any) => { if (e.relatedTarget) { @@ -162,7 +163,7 @@ export default function useQuickInput() { }; useKeyboardShortcuts({ - [openQuickInputShortcutKey]: () => { + [openCommandBarShortcutAction]: () => { quickInputService.show(); }, }); @@ -193,7 +194,7 @@ export default function useQuickInput() { onInputChange: quickInputService.setInputValue, onHide: quickInputService.hide, onShow: quickInputService.show, - keyboardShortcut: getShortcut(openQuickInputShortcutKey), + keyboardShortcut: getAccelerator(openCommandBarShortcutAction), }; } diff --git a/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx b/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx index 606dab1dd9930..5a120f485583a 100644 --- a/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx +++ b/web/packages/teleterm/src/ui/TabHost/TabHost.test.tsx @@ -67,12 +67,12 @@ function getTestSetup({ documents }: { documents: Document[] }) { // @ts-expect-error we don't provide entire config getShortcutsConfig() { return { - 'tab-close': 'Command-W', - 'tab-new': 'Command-T', - 'open-quick-input': 'Command-K', - 'toggle-connections': 'Command-P', - 'toggle-clusters': 'Command-E', - 'toggle-identity': 'Command-I', + closeTab: 'Command-W', + newTab: 'Command-T', + openCommandBar: 'Command-K', + openConnections: 'Command-P', + openClusters: 'Command-E', + openProfiles: 'Command-I', }; }, }; diff --git a/web/packages/teleterm/src/ui/TabHost/TabHost.tsx b/web/packages/teleterm/src/ui/TabHost/TabHost.tsx index 51d19782827f8..fdc544e0a49d9 100644 --- a/web/packages/teleterm/src/ui/TabHost/TabHost.tsx +++ b/web/packages/teleterm/src/ui/TabHost/TabHost.tsx @@ -51,7 +51,7 @@ export function TabHost({ ctx }: { ctx: IAppContext }) { localClusterUri: ctx.workspacesService.getActiveWorkspace()?.localClusterUri, }); - const { getLabelWithShortcut } = useKeyboardShortcutFormatters(); + const { getLabelWithAccelerator } = useKeyboardShortcutFormatters(); useTabShortcuts({ documentsService, @@ -108,8 +108,8 @@ export function TabHost({ ctx }: { ctx: IAppContext }) { onMoved={handleTabMoved} disableNew={false} onNew={openClusterTab} - newTabTooltip={getLabelWithShortcut('New Tab', 'tab-new')} - closeTabTooltip={getLabelWithShortcut('Close', 'tab-close')} + newTabTooltip={getLabelWithAccelerator('New Tab', 'newTab')} + closeTabTooltip={getLabelWithAccelerator('Close', 'closeTab')} /> diff --git a/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx b/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx index d6b75f1bb88a3..2960113e6c6ec 100644 --- a/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx +++ b/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.test.tsx @@ -177,21 +177,21 @@ function getTestSetup({ documents }: { documents: Document[] }) { } test.each([ - { type: 'tab-1', value: 0 }, - { type: 'tab-2', value: 1 }, - { type: 'tab-3', value: 2 }, - { type: 'tab-4', value: 3 }, - { type: 'tab-5', value: 4 }, - { type: 'tab-6', value: 5 }, - { type: 'tab-7', value: 6 }, - { type: 'tab-8', value: 7 }, - { type: 'tab-9', value: 8 }, -])('open tab using $type shortcut', ({ type, value }) => { + { action: 'tab1', value: 0 }, + { action: 'tab2', value: 1 }, + { action: 'tab3', value: 2 }, + { action: 'tab4', value: 3 }, + { action: 'tab5', value: 4 }, + { action: 'tab6', value: 5 }, + { action: 'tab7', value: 6 }, + { action: 'tab8', value: 7 }, + { action: 'tab9', value: 8 }, +])('open tab using $type shortcut', ({ action, value }) => { const { emitKeyboardShortcutEvent, docsService } = getTestSetup({ documents: getMockDocuments(), }); - emitKeyboardShortcutEvent({ type } as KeyboardShortcutEvent); + emitKeyboardShortcutEvent({ action } as KeyboardShortcutEvent); expect(docsService.open).toHaveBeenCalledWith( docsService.getDocuments()[value].uri @@ -205,7 +205,7 @@ test('close active tab', () => { const documentToClose = docsService.getDocuments()[0]; docsService.getActive = () => documentToClose; - emitKeyboardShortcutEvent({ type: 'tab-close' }); + emitKeyboardShortcutEvent({ action: 'closeTab' }); expect(docsService.close).toHaveBeenCalledWith(documentToClose.uri); }); @@ -215,7 +215,7 @@ test('should ignore close command if no tabs are open', () => { documents: [], }); - emitKeyboardShortcutEvent({ type: 'tab-close' }); + emitKeyboardShortcutEvent({ action: 'closeTab' }); expect(docsService.close).not.toHaveBeenCalled(); }); @@ -231,7 +231,7 @@ test('open new tab', () => { kind: 'doc.cluster', }; docsService.createClusterDocument = () => mockedClusterDocument; - emitKeyboardShortcutEvent({ type: 'tab-new' }); + emitKeyboardShortcutEvent({ action: 'newTab' }); expect(docsService.add).toHaveBeenCalledWith(mockedClusterDocument); expect(docsService.open).toHaveBeenCalledWith(mockedClusterDocument.uri); @@ -245,7 +245,7 @@ describe('open next/previous tab', () => { const activeTabIndex = 2; docsService.getActive = () => docsService.getDocuments()[activeTabIndex]; - emitKeyboardShortcutEvent({ type: 'tab-next' }); + emitKeyboardShortcutEvent({ action: 'nextTab' }); expect(docsService.open).toHaveBeenCalledWith( docsService.getDocuments()[activeTabIndex + 1].uri @@ -259,7 +259,7 @@ describe('open next/previous tab', () => { const activeTabIndex = docsService.getDocuments().length - 1; docsService.getActive = () => docsService.getDocuments()[activeTabIndex]; - emitKeyboardShortcutEvent({ type: 'tab-next' }); + emitKeyboardShortcutEvent({ action: 'nextTab' }); expect(docsService.open).toHaveBeenCalledWith( docsService.getDocuments()[0].uri @@ -273,7 +273,7 @@ describe('open next/previous tab', () => { const activeTabIndex = 2; docsService.getActive = () => docsService.getDocuments()[activeTabIndex]; - emitKeyboardShortcutEvent({ type: 'tab-previous' }); + emitKeyboardShortcutEvent({ action: 'previousTab' }); expect(docsService.open).toHaveBeenCalledWith( docsService.getDocuments()[activeTabIndex - 1].uri @@ -287,7 +287,7 @@ describe('open next/previous tab', () => { const activeTabIndex = 0; docsService.getActive = () => docsService.getDocuments()[activeTabIndex]; - emitKeyboardShortcutEvent({ type: 'tab-previous' }); + emitKeyboardShortcutEvent({ action: 'previousTab' }); expect(docsService.open).toHaveBeenCalledWith( docsService.getDocuments()[docsService.getDocuments().length - 1].uri @@ -298,7 +298,7 @@ describe('open next/previous tab', () => { const { emitKeyboardShortcutEvent, docsService } = getTestSetup({ documents: [], }); - emitKeyboardShortcutEvent({ type: 'tab-next' }); + emitKeyboardShortcutEvent({ action: 'nextTab' }); expect(docsService.open).not.toHaveBeenCalled(); }); diff --git a/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.ts b/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.ts index 36afb53ee20d0..74f14b962d80c 100644 --- a/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.ts +++ b/web/packages/teleterm/src/ui/TabHost/useTabShortcuts.ts @@ -78,18 +78,18 @@ function buildTabsShortcuts( documentService.open(allDocuments[indexToOpen].uri); }; return { - 'tab-1': handleTabIndex(0), - 'tab-2': handleTabIndex(1), - 'tab-3': handleTabIndex(2), - 'tab-4': handleTabIndex(3), - 'tab-5': handleTabIndex(4), - 'tab-6': handleTabIndex(5), - 'tab-7': handleTabIndex(6), - 'tab-8': handleTabIndex(7), - 'tab-9': handleTabIndex(8), - 'tab-close': handleActiveTabClose, - 'tab-previous': handleTabSwitch('previous'), - 'tab-next': handleTabSwitch('next'), - 'tab-new': openClusterTab, + tab1: handleTabIndex(0), + tab2: handleTabIndex(1), + tab3: handleTabIndex(2), + tab4: handleTabIndex(3), + tab5: handleTabIndex(4), + tab6: handleTabIndex(5), + tab7: handleTabIndex(6), + tab8: handleTabIndex(7), + tab9: handleTabIndex(8), + closeTab: handleActiveTabClose, + previousTab: handleTabSwitch('previous'), + nextTab: handleTabSwitch('next'), + newTab: openClusterTab, }; } diff --git a/web/packages/teleterm/src/ui/TopBar/Clusters/ClusterSelector/ClusterSelector.tsx b/web/packages/teleterm/src/ui/TopBar/Clusters/ClusterSelector/ClusterSelector.tsx index 66f2bfd635e75..4342cd61bfa11 100644 --- a/web/packages/teleterm/src/ui/TopBar/Clusters/ClusterSelector/ClusterSelector.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Clusters/ClusterSelector/ClusterSelector.tsx @@ -30,7 +30,7 @@ interface ClusterSelectorProps { export const ClusterSelector = forwardRef( (props, ref) => { - const { getLabelWithShortcut } = useKeyboardShortcutFormatters(); + const { getLabelWithAccelerator } = useKeyboardShortcutFormatters(); const SortIcon = props.isOpened ? SortAsc : SortDesc; const text = props.clusterName || 'Select Cluster'; @@ -40,9 +40,9 @@ export const ClusterSelector = forwardRef( onClick={props.onClick} isOpened={props.isOpened} isClusterSelected={!!props.clusterName} - title={getLabelWithShortcut( + title={getLabelWithAccelerator( [props.clusterName, 'Open Clusters'].filter(Boolean).join('\n'), - 'toggle-clusters' + 'openClusters' )} > ({ - 'toggle-clusters': togglePopover, + openClusters: togglePopover, }), [togglePopover] ) diff --git a/web/packages/teleterm/src/ui/TopBar/Connections/Connections.tsx b/web/packages/teleterm/src/ui/TopBar/Connections/Connections.tsx index 28ce96225ab8e..73fd1007a0a11 100644 --- a/web/packages/teleterm/src/ui/TopBar/Connections/Connections.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Connections/Connections.tsx @@ -45,7 +45,7 @@ export function Connections() { useKeyboardShortcuts( useMemo( () => ({ - 'toggle-connections': togglePopover, + openConnections: togglePopover, }), [togglePopover] ) diff --git a/web/packages/teleterm/src/ui/TopBar/Connections/ConnectionsIcon/ConnectionsIcon.tsx b/web/packages/teleterm/src/ui/TopBar/Connections/ConnectionsIcon/ConnectionsIcon.tsx index c1cd24bacbd8b..cdae59d4a4ee0 100644 --- a/web/packages/teleterm/src/ui/TopBar/Connections/ConnectionsIcon/ConnectionsIcon.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Connections/ConnectionsIcon/ConnectionsIcon.tsx @@ -31,7 +31,7 @@ interface ConnectionsIconProps { export const ConnectionsIcon = forwardRef( (props, ref) => { - const { getLabelWithShortcut } = useKeyboardShortcutFormatters(); + const { getLabelWithAccelerator } = useKeyboardShortcutFormatters(); return ( ( kind="secondary" size="small" m="auto" - title={getLabelWithShortcut('Open Connections', 'toggle-connections')} + title={getLabelWithAccelerator('Open Connections', 'openConnections')} > diff --git a/web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx b/web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx index 80722fdd1071a..831a87ea699c7 100644 --- a/web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx +++ b/web/packages/teleterm/src/ui/TopBar/Identity/Identity.tsx @@ -45,23 +45,23 @@ export function IdentityContainer() { logout, addCluster, } = useIdentity(); - const { getLabelWithShortcut } = useKeyboardShortcutFormatters(); + const { getLabelWithAccelerator } = useKeyboardShortcutFormatters(); const presenterRef = useRef(); useKeyboardShortcuts( useMemo( () => ({ - 'toggle-identity': presenterRef.current?.togglePopover, + openProfiles: presenterRef.current?.togglePopover, }), [presenterRef.current?.togglePopover] ) ); const makeTitle = (userWithClusterName: string | undefined) => - getLabelWithShortcut( + getLabelWithAccelerator( [userWithClusterName, 'Open Profiles'].filter(Boolean).join('\n'), - 'toggle-identity' + 'openProfiles' ); return ( diff --git a/web/packages/teleterm/src/ui/TopBar/NavigationMenu/NavigationItem.tsx b/web/packages/teleterm/src/ui/TopBar/NavigationMenu/NavigationItem.tsx index 10ebd0f2d8815..2674572f67e63 100644 --- a/web/packages/teleterm/src/ui/TopBar/NavigationMenu/NavigationItem.tsx +++ b/web/packages/teleterm/src/ui/TopBar/NavigationMenu/NavigationItem.tsx @@ -17,7 +17,9 @@ import React from 'react'; import styled from 'styled-components'; -import { Box, Text, Flex } from 'design'; +import { Text } from 'design'; + +import { ListItem } from 'teleterm/ui/components/ListItem'; export function NavigationItem({ item, closeMenu }: NavigationItemProps) { const handleClick = () => { @@ -26,34 +28,24 @@ export function NavigationItem({ item, closeMenu }: NavigationItemProps) { }; return ( - - {item.Icon} + + {item.title} - + ); } -const ListItem = styled(Flex)` - border-radius: 4px; - align-items: center; - justify-content: start; - cursor: pointer; - &:hover { - background: ${props => props.theme.colors.primary.lighter}; - } -`; - -const IconBox = styled(Box)` - display: flex; - align-items: center; - border-radius: 4px; - background-color: ${props => props.theme.colors.primary.lighter}; +const StyledListItem = styled(ListItem)` + height: 38px; + gap: 12px; + padding: 0 12px; + border-radius: 0; `; type NavigationItemProps = { item: { title: string; - Icon: JSX.Element; + Icon: React.ComponentType<{ fontSize: number }>; onNavigate: () => void; }; closeMenu: () => void; diff --git a/web/packages/teleterm/src/ui/TopBar/NavigationMenu/NavigationMenu.tsx b/web/packages/teleterm/src/ui/TopBar/NavigationMenu/NavigationMenu.tsx index 18fdb23939986..1c641e691253c 100644 --- a/web/packages/teleterm/src/ui/TopBar/NavigationMenu/NavigationMenu.tsx +++ b/web/packages/teleterm/src/ui/TopBar/NavigationMenu/NavigationMenu.tsx @@ -18,75 +18,108 @@ import React, { useState, useRef } from 'react'; import styled from 'styled-components'; import Popover from 'design/Popover'; -import { MoreVert, OpenBox, Add } from 'design/Icon'; -import { Box, Text, Flex } from 'design'; +import { MoreVert, OpenBox, Add, Config } from 'design/Icon'; import { useAppContext } from 'teleterm/ui/appContextProvider'; -import { DocumentsService } from 'teleterm/ui/services/workspacesService'; import { TopBarButton } from 'teleterm/ui/TopBar/TopBarButton'; -import { RootClusterUri } from 'teleterm/ui/uri'; - -import { useIdentity } from '../Identity/useIdentity'; +import { IAppContext } from 'teleterm/ui/types'; +import { Cluster } from 'teleterm/services/tshd/types'; import { NavigationItem } from './NavigationItem'; -function getNavigationItems( - documentsService: DocumentsService, - clusterUri: RootClusterUri -): { - title: string; - Icon: JSX.Element; - onNavigate: () => void; -}[] { +function useNavigationItems(): ( + | { + title: string; + Icon: React.ComponentType<{ fontSize: number }>; + onNavigate: () => void; + } + | 'separator' +)[] { + const ctx = useAppContext(); + ctx.workspacesService.useState(); + ctx.clustersService.useState(); + + const documentsService = + ctx.workspacesService.getActiveWorkspaceDocumentService(); + const activeRootCluster = getActiveRootCluster(ctx); + const areAccessRequestsSupported = + !!activeRootCluster?.features?.advancedAccessWorkflows; + return [ { - title: 'New Access Request', - Icon: , - onNavigate: () => { - const doc = documentsService.createAccessRequestDocument({ - clusterUri, - state: 'creating', - title: 'New Access Request', - }); - documentsService.add(doc); - documentsService.open(doc.uri); + title: 'Open Config File', + Icon: Config, + onNavigate: async () => { + const path = await ctx.mainProcessClient.openConfigFile(); + ctx.notificationsService.notifyInfo( + `Opened the config file at ${path}.` + ); }, }, - { - title: 'Review Access Requests', - Icon: , - onNavigate: () => { - const doc = documentsService.createAccessRequestDocument({ - clusterUri, - state: 'browsing', - }); - documentsService.add(doc); - documentsService.open(doc.uri); - }, - }, - ]; + ...(areAccessRequestsSupported + ? [ + 'separator' as const, + { + title: 'New Access Request', + Icon: Add, + onNavigate: () => { + const doc = documentsService.createAccessRequestDocument({ + clusterUri: activeRootCluster.uri, + state: 'creating', + title: 'New Access Request', + }); + documentsService.add(doc); + documentsService.open(doc.uri); + }, + }, + { + title: 'Review Access Requests', + Icon: OpenBox, + onNavigate: () => { + const doc = documentsService.createAccessRequestDocument({ + clusterUri: activeRootCluster.uri, + state: 'browsing', + }); + documentsService.add(doc); + documentsService.open(doc.uri); + }, + }, + ] + : []), + ].filter(Boolean); } -export function NavigationMenu() { - const ctx = useAppContext(); - const documentsService = - ctx.workspacesService.getActiveWorkspaceDocumentService(); - const { activeRootCluster } = useIdentity(); +function getActiveRootCluster(ctx: IAppContext): Cluster | undefined { + const clusterUri = ctx.workspacesService.getRootClusterUri(); + if (!clusterUri) { + return; + } + return ctx.clustersService.findCluster(clusterUri); +} +export function NavigationMenu() { const [isPopoverOpened, setIsPopoverOpened] = useState(false); const selectorRef = useRef(); - const shouldShowMenu = !!activeRootCluster?.features?.advancedAccessWorkflows; - if (!shouldShowMenu) { - return null; - } + const items = useNavigationItems().map((item, index) => { + if (item === 'separator') { + return ; + } + return ( + setIsPopoverOpened(false)} + /> + ); + }); return ( <> setIsPopoverOpened(true)} > @@ -99,27 +132,23 @@ export function NavigationMenu() { onClose={() => setIsPopoverOpened(false)} popoverCss={() => `max-width: min(560px, 90%)`} > - - - Go To - - {getNavigationItems(documentsService, activeRootCluster.uri).map( - (item, index) => ( - setIsPopoverOpened(false)} - /> - ) - )} - - - + {items} ); } -const MenuContainer = styled(Box)` +const Menu = styled.menu` + list-style: none; + padding: 0; + margin: 0; + display: flex; + flex-direction: column; + min-width: 280px; background: ${props => props.theme.colors.primary.light}; `; + +const Separator = styled.div` + background: ${props => props.theme.colors.primary.lighter}; + height: 1px; +`; diff --git a/web/packages/teleterm/src/ui/initUi.test.ts b/web/packages/teleterm/src/ui/initUi.test.ts index 3cd2ce87a6420..4e73331fc14e0 100644 --- a/web/packages/teleterm/src/ui/initUi.test.ts +++ b/web/packages/teleterm/src/ui/initUi.test.ts @@ -109,6 +109,25 @@ describe('usage reporting dialogs', () => { }); }); +test('no dialog is shown when config file did not load properly', async () => { + const mockedAppContext = new MockAppContext(); + jest + .spyOn(mockedAppContext.mainProcessClient.configService, 'getConfigError') + .mockImplementation(() => ({ source: 'file-loading', error: new Error() })); + mockOpenRegularDialog(mockedAppContext); + + await initUi(mockedAppContext); + + expect( + mockedAppContext.modalsService.openRegularDialog + ).not.toHaveBeenCalledWith(expect.objectContaining({ kind: 'usage-data' })); + expect( + mockedAppContext.modalsService.openRegularDialog + ).not.toHaveBeenCalledWith( + expect.objectContaining({ kind: 'user-job-role' }) + ); +}); + function mockUsageReportingEnabled( mockedAppContext: AppContext, options: { enabled: boolean } diff --git a/web/packages/teleterm/src/ui/initUi.ts b/web/packages/teleterm/src/ui/initUi.ts index 4a39fae981356..3bd212184110a 100644 --- a/web/packages/teleterm/src/ui/initUi.ts +++ b/web/packages/teleterm/src/ui/initUi.ts @@ -21,6 +21,7 @@ import { import { IAppContext } from 'teleterm/ui/types'; import { ConfigService } from 'teleterm/services/config'; import { NotificationsService } from 'teleterm/ui/services/notifications'; +import { KeyboardShortcutsService } from 'teleterm/ui/services/keyboardShortcuts'; /** * Runs after the UI becomes visible. @@ -42,20 +43,63 @@ export async function initUi(ctx: IAppContext): Promise { // "User job role" dialog is shown on the second launch (only if user agreed to reporting earlier). await setUpUsageReporting(configService, ctx.modalsService); ctx.workspacesService.restorePersistedState(); - notifyAboutStoredConfigErrors(configService, ctx.notificationsService); + notifyAboutConfigErrors(configService, ctx.notificationsService); + notifyAboutDuplicatedShortcutsCombinations( + ctx.keyboardShortcutsService, + ctx.notificationsService + ); } -function notifyAboutStoredConfigErrors( +function notifyAboutConfigErrors( configService: ConfigService, notificationsService: NotificationsService ): void { - const errors = configService.getStoredConfigErrors(); - if (errors) { + const configError = configService.getConfigError(); + if (configError) { + switch (configError.source) { + case 'file-loading': { + notificationsService.notifyError({ + title: 'Failed to load config file', + description: `Using the default config instead.\n${configError.error}`, + }); + break; + } + case 'validation': { + const isKeymapError = configError.errors.some(e => + e.path[0].toString().startsWith('keymap.') + ); + notificationsService.notifyError({ + title: 'Encountered errors in config file', + list: configError.errors.map( + e => `${e.path[0].toString()}: ${e.message}` + ), + description: + isKeymapError && + 'A valid shortcut contains at least one modifier and a single key code, for example "Shift+Tab".\nFunction keys do not require a modifier.', + link: { + href: 'https://goteleport.com/docs/connect-your-client/teleport-connect/#configuration', + text: 'See the config file documentation', + }, + }); + } + } + } +} + +function notifyAboutDuplicatedShortcutsCombinations( + keyboardShortcutsService: KeyboardShortcutsService, + notificationsService: NotificationsService +): void { + const duplicates = keyboardShortcutsService.getDuplicateAccelerators(); + if (Object.keys(duplicates).length) { notificationsService.notifyError({ - title: 'Encountered errors in config file', - description: errors - .map(error => `${error.path[0]}: ${error.message}`) - .join('\n'), + title: 'Shortcuts conflicts', + list: Object.entries(duplicates).map( + ([accelerator, actions]) => + `${accelerator} is used for actions: ${actions.join( + ', ' + )}. Only one of them will work.` + ), }); } } diff --git a/web/packages/teleterm/src/ui/services/keyboardShortcuts/getKeyName.ts b/web/packages/teleterm/src/ui/services/keyboardShortcuts/getKeyName.ts new file mode 100644 index 0000000000000..8835210e2ff20 --- /dev/null +++ b/web/packages/teleterm/src/ui/services/keyboardShortcuts/getKeyName.ts @@ -0,0 +1,85 @@ +/** + * Copyright 2023 Gravitational, Inc + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Based on tabby https://github.com/Eugeny/tabby/blob/7a8108b20d9cbaab636c932ceaf4bacc710d6a40/tabby-core/src/services/hotkeys.util.ts + * + * The MIT License (MIT) + * + * Copyright (c) 2017 Eugene Pankov + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + */ + +const REGEX_LATIN_KEYNAME = /^[A-Za-z]$/; + +/** + * Returns a key name in a way that makes some keys independent of their physical placement on the US QWERTY layout. + * + * First, we check if the printed character is from the range A-Z (case-insensitive). + * This check bounds the letters to the (changeable) keyboard layout, not the physical keys. + * For example, in the Dvorak layout, the "K" and "T" keys are interchanged (compared to US QWERTY). + * By relying on the printed character, we are independent of the layout. + * This regex also excludes non-Latin characters, which should be handled by physical codes, + * because `KeyboardEvent.key` will be a letter from that alphabet. + * Most of these keyboards follow the standard US QWERTY layout, + * so it is possible for `KeyboardEvent.code` to work as a fallback. + * + * The rest of the keys are handled by their physical code. + * It is common in many layouts that the user has to press a modifier to input a character + * which is available on US QWERTY without any modifiers. + * For example, in Czech QWERTY there is no "1" key (it is on upper row) - + * the user has to press "Shift+(plus key)" to get "1". + * The resulting character is still "1" as in US QWERTY, + * but because "Shift" was pressed we would interpret it as a different shortcut ("Shift+1", not "1"). + * + * The above mechanism is not perfect, because only A-Z keys are mapped to the active layout. + * Keys like comma are still tied to the physical keys in the US QWERTY. + * */ +export function getKeyName(event: KeyboardEvent): string { + if (REGEX_LATIN_KEYNAME.test(event.key)) { + // Handle Dvorak etc. via the reported "character" instead of the scancode + return event.key.toUpperCase(); + } + let key = event.code; + key = key.replace('Key', ''); + key = key.replace('Arrow', ''); + key = key.replace('Digit', ''); + key = PHYSICAL_CODE_TO_PRINTABLE_CHARACTER[key] ?? key; + return key; +} + +const PHYSICAL_CODE_TO_PRINTABLE_CHARACTER = { + Comma: ',', + Period: '.', + Slash: '/', + Backslash: '\\', + Backquote: '`', + Minus: '-', + Equal: '=', + Semicolon: ';', + Quote: "'", + BracketLeft: '[', + BracketRight: ']', +}; diff --git a/web/packages/teleterm/src/ui/services/keyboardShortcuts/keyboardShortcutsService.test.ts b/web/packages/teleterm/src/ui/services/keyboardShortcuts/keyboardShortcutsService.test.ts index 91a3bf25634a0..ed91061488594 100644 --- a/web/packages/teleterm/src/ui/services/keyboardShortcuts/keyboardShortcutsService.test.ts +++ b/web/packages/teleterm/src/ui/services/keyboardShortcuts/keyboardShortcutsService.test.ts @@ -21,7 +21,7 @@ import { KeyboardShortcutsService } from './keyboardShortcutsService'; test('call subscriber on event', () => { const { subscriber } = getTestSetup(); dispatchEventCommand1(); - expect(subscriber).toHaveBeenCalledWith({ type: 'tab-1' }); + expect(subscriber).toHaveBeenCalledWith({ action: 'tab1' }); }); test('do not call subscriber on unknown event', () => { @@ -39,10 +39,25 @@ test('do not call subscriber after it has been unsubscribed', () => { expect(subscriber).not.toHaveBeenCalled(); }); +test('duplicate accelerators are returned', () => { + const service = new KeyboardShortcutsService( + 'darwin', + createMockConfigService({ + 'keymap.tab1': 'Command+1', + 'keymap.tab2': 'Command+1', + 'keymap.tab3': 'Command+2', + }) + ); + + expect(service.getDuplicateAccelerators()).toStrictEqual({ + 'Command+1': ['tab1', 'tab2'], + }); +}); + function getTestSetup() { const service = new KeyboardShortcutsService( 'darwin', - createMockConfigService({ 'keymap.tab1': 'Command-1' }) + createMockConfigService({ 'keymap.tab1': 'Command+1' }) ); const subscriber = jest.fn(); service.subscribeToEvents(subscriber); @@ -50,5 +65,5 @@ function getTestSetup() { } function dispatchEventCommand1() { - dispatchEvent(new KeyboardEvent('keydown', { metaKey: true, key: '1' })); + dispatchEvent(new KeyboardEvent('keydown', { metaKey: true, code: '1' })); } diff --git a/web/packages/teleterm/src/ui/services/keyboardShortcuts/keyboardShortcutsService.ts b/web/packages/teleterm/src/ui/services/keyboardShortcuts/keyboardShortcutsService.ts index 1be88de80b3ba..7eb300c67a961 100644 --- a/web/packages/teleterm/src/ui/services/keyboardShortcuts/keyboardShortcutsService.ts +++ b/web/packages/teleterm/src/ui/services/keyboardShortcuts/keyboardShortcutsService.ts @@ -16,11 +16,11 @@ import { Platform } from 'teleterm/mainProcess/types'; import { - KeyboardShortcutsConfig, - KeyboardShortcutType, + KeyboardShortcutAction, ConfigService, } from 'teleterm/services/config'; +import { getKeyName } from './getKeyName'; import { KeyboardShortcutEvent, KeyboardShortcutEventSubscriber, @@ -28,34 +28,41 @@ import { export class KeyboardShortcutsService { private eventsSubscribers = new Set(); - private keysToShortcuts = new Map(); - private readonly shortcutsConfig: Record; + private readonly acceleratorsToActions = new Map< + string, + KeyboardShortcutAction[] + >(); + /** + * Modifier keys must be defined in the following order: + * Control-Option-Shift-Command for macOS + * Ctrl-Alt-Shift for other platforms + */ + private readonly shortcutsConfig: Record; constructor( private platform: Platform, private configService: ConfigService ) { this.shortcutsConfig = { - 'tab-1': this.configService.get('keymap.tab1').value, - 'tab-2': this.configService.get('keymap.tab2').value, - 'tab-3': this.configService.get('keymap.tab3').value, - 'tab-4': this.configService.get('keymap.tab4').value, - 'tab-5': this.configService.get('keymap.tab5').value, - 'tab-6': this.configService.get('keymap.tab6').value, - 'tab-7': this.configService.get('keymap.tab7').value, - 'tab-8': this.configService.get('keymap.tab8').value, - 'tab-9': this.configService.get('keymap.tab9').value, - 'tab-close': this.configService.get('keymap.tabClose').value, - 'tab-previous': this.configService.get('keymap.tabPrevious').value, - 'tab-next': this.configService.get('keymap.tabNext').value, - 'tab-new': this.configService.get('keymap.tabNew').value, - 'open-quick-input': this.configService.get('keymap.openQuickInput').value, - 'toggle-connections': this.configService.get('keymap.toggleConnections') - .value, - 'toggle-clusters': this.configService.get('keymap.toggleClusters').value, - 'toggle-identity': this.configService.get('keymap.toggleIdentity').value, + tab1: this.configService.get('keymap.tab1').value, + tab2: this.configService.get('keymap.tab2').value, + tab3: this.configService.get('keymap.tab3').value, + tab4: this.configService.get('keymap.tab4').value, + tab5: this.configService.get('keymap.tab5').value, + tab6: this.configService.get('keymap.tab6').value, + tab7: this.configService.get('keymap.tab7').value, + tab8: this.configService.get('keymap.tab8').value, + tab9: this.configService.get('keymap.tab9').value, + closeTab: this.configService.get('keymap.closeTab').value, + previousTab: this.configService.get('keymap.previousTab').value, + nextTab: this.configService.get('keymap.nextTab').value, + newTab: this.configService.get('keymap.newTab').value, + openCommandBar: this.configService.get('keymap.openCommandBar').value, + openConnections: this.configService.get('keymap.openConnections').value, + openClusters: this.configService.get('keymap.openClusters').value, + openProfiles: this.configService.get('keymap.openProfiles').value, }; - this.recalculateKeysToShortcuts(this.shortcutsConfig); + this.acceleratorsToActions = mapAcceleratorsToActions(this.shortcutsConfig); this.attachKeydownHandler(); } @@ -71,16 +78,32 @@ export class KeyboardShortcutsService { return this.shortcutsConfig; } + /** + * Some actions can get assigned the same accelerators. + * This method returns them. + */ + getDuplicateAccelerators(): Record { + return Array.from(this.acceleratorsToActions.entries()) + .filter(([, shortcuts]) => shortcuts.length > 1) + .reduce>( + (accumulator, [accelerator, actions]) => { + accumulator[accelerator] = actions; + return accumulator; + }, + {} + ); + } + private attachKeydownHandler(): void { const handleKeydown = (event: KeyboardEvent): void => { - const shortcutType = this.getShortcut(event); - if (!shortcutType) { + const shortcutAction = this.getShortcutAction(event); + if (!shortcutAction) { return; } event.preventDefault(); event.stopPropagation(); - this.notifyEventsSubscribers({ type: shortcutType }); + this.notifyEventsSubscribers({ action: shortcutAction }); }; window.addEventListener('keydown', handleKeydown, { @@ -88,25 +111,43 @@ export class KeyboardShortcutsService { }); } - private getShortcut(event: KeyboardEvent): KeyboardShortcutType | undefined { - const getEventKey = () => - event.key.length === 1 ? event.key.toUpperCase() : event.key; - - const key = [...this.getPlatformModifierKeys(event), getEventKey()] + private getShortcutAction( + event: KeyboardEvent + ): KeyboardShortcutAction | undefined { + // If only a modifier is pressed, `code` is this modifier name + // (in case of a combination like "Cmd+A", `code` is "A"). + // We do not support modifier-only accelerators, so we can skip the further checks. + if ( + event.code.includes('Shift') || + event.code.includes('Meta') || + event.code.includes('Alt') || + event.code.includes('Control') + ) { + return; + } + const accelerator = [ + ...this.getPlatformModifierKeys(event), + getKeyName(event), + ] .filter(Boolean) - .join('-'); + .join('+'); - return this.keysToShortcuts.get(key); + // always return the first action (in case of duplicate accelerators) + return this.acceleratorsToActions.get(accelerator)?.[0]; } + /** + * It is important that these modifiers are in the same order as in `getKeyboardShortcutSchema#getSupportedModifiers`. + * Consider creating "one source of truth" for them. + */ private getPlatformModifierKeys(event: KeyboardEvent): string[] { switch (this.platform) { case 'darwin': return [ - event.metaKey && 'Command', event.ctrlKey && 'Control', event.altKey && 'Option', event.shiftKey && 'Shift', + event.metaKey && 'Command', ]; default: return [ @@ -117,19 +158,25 @@ export class KeyboardShortcutsService { } } - /** - * Inverts shortcuts-keys pairs to allow accessing shortcut by a key - */ - private recalculateKeysToShortcuts( - toInvert: Partial - ): void { - this.keysToShortcuts.clear(); - Object.entries(toInvert).forEach(([shortcutType, key]) => { - this.keysToShortcuts.set(key, shortcutType as KeyboardShortcutType); - }); - } - private notifyEventsSubscribers(event: KeyboardShortcutEvent): void { this.eventsSubscribers.forEach(subscriber => subscriber(event)); } } + +/** Inverts shortcuts-keys pairs to allow accessing shortcut by an accelerator. */ +function mapAcceleratorsToActions( + shortcutsConfig: Record +): Map { + const acceleratorsToActions = new Map(); + Object.entries(shortcutsConfig).forEach(([action, accelerator]) => { + // empty accelerator means that an empty string was provided in the config file, so the shortcut is disabled. + if (!accelerator) { + return; + } + acceleratorsToActions.set(accelerator, [ + ...(acceleratorsToActions.get(accelerator) || []), + action as KeyboardShortcutAction, + ]); + }); + return acceleratorsToActions; +} diff --git a/web/packages/teleterm/src/ui/services/keyboardShortcuts/types.ts b/web/packages/teleterm/src/ui/services/keyboardShortcuts/types.ts index ce6142925cbb9..b5fa6b864475b 100644 --- a/web/packages/teleterm/src/ui/services/keyboardShortcuts/types.ts +++ b/web/packages/teleterm/src/ui/services/keyboardShortcuts/types.ts @@ -14,10 +14,10 @@ * limitations under the License. */ -import { KeyboardShortcutType } from 'teleterm/services/config'; +import { KeyboardShortcutAction } from 'teleterm/services/config'; export interface KeyboardShortcutEvent { - type: KeyboardShortcutType; + action: KeyboardShortcutAction; } export type KeyboardShortcutEventSubscriber = ( @@ -25,5 +25,5 @@ export type KeyboardShortcutEventSubscriber = ( ) => void; export type KeyboardShortcutHandlers = Partial< - Record void> + Record void> >; diff --git a/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcutFormatters.ts b/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcutFormatters.ts index 94e232eed4ef4..5d929404aef77 100644 --- a/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcutFormatters.ts +++ b/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcutFormatters.ts @@ -14,19 +14,19 @@ * limitations under the License. */ -import { KeyboardShortcutType } from '../../../services/config'; +import { KeyboardShortcutAction } from '../../../services/config'; import { useAppContext } from '../../appContextProvider'; import { Platform } from '../../../mainProcess/types'; interface KeyboardShortcutFormatters { - getLabelWithShortcut( + getLabelWithAccelerator( label: string, - shortcutKey: KeyboardShortcutType, + action: KeyboardShortcutAction, options?: KeyboardShortcutFormattingOptions ): string; - getShortcut( - shortcutKey: KeyboardShortcutType, + getAccelerator( + action: KeyboardShortcutAction, options?: KeyboardShortcutFormattingOptions ): string; } @@ -41,41 +41,40 @@ export function useKeyboardShortcutFormatters(): KeyboardShortcutFormatters { const keyboardShortcuts = keyboardShortcutsService.getShortcutsConfig(); return { - getLabelWithShortcut(label, shortcutKey, options) { - const formattedShortcut = formatKeyboardShortcut({ + getLabelWithAccelerator(label, action, options) { + const formattedAccelerator = formatAccelerator({ platform, - shortcutValue: keyboardShortcuts[shortcutKey], + accelerator: keyboardShortcuts[action], ...options, }); - return `${label} (${formattedShortcut})`; + return `${label} (${formattedAccelerator})`; }, - getShortcut(shortcutKey, options) { - return formatKeyboardShortcut({ + getAccelerator(action, options) { + return formatAccelerator({ platform, - shortcutValue: keyboardShortcuts[shortcutKey], + accelerator: keyboardShortcuts[action], ...options, }); }, }; } -function formatKeyboardShortcut(options: { +function formatAccelerator(options: { platform: Platform; - shortcutValue: string; + accelerator: string; useWhitespaceSeparator?: boolean; }): string { switch (options.platform) { case 'darwin': - return options.shortcutValue - .replace('-', options.useWhitespaceSeparator ? ' ' : '') + return options.accelerator + .replaceAll('+', options.useWhitespaceSeparator ? ' ' : '') .replace('Command', '⌘') .replace('Control', '⌃') .replace('Option', '⌥') .replace('Shift', '⇧'); default: - return options.shortcutValue.replace( - '-', - options.useWhitespaceSeparator ? ' + ' : '+' - ); + return options.useWhitespaceSeparator + ? options.accelerator.replaceAll('+', ' + ') + : options.accelerator; } } diff --git a/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcuts.test.tsx b/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcuts.test.tsx index 59b18206a588f..84477287aa340 100644 --- a/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcuts.test.tsx +++ b/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcuts.test.tsx @@ -30,9 +30,9 @@ import { KeyboardShortcutEventSubscriber } from './types'; test('call handler on its event type', () => { const { handler, getEventEmitter, wrapper } = getTestSetup(); - renderHook(() => useKeyboardShortcuts({ 'tab-1': handler }), { wrapper }); + renderHook(() => useKeyboardShortcuts({ tab1: handler }), { wrapper }); const emitEvent = getEventEmitter(); - emitEvent({ type: 'tab-1' }); + emitEvent({ action: 'tab1' }); expect(handler).toHaveBeenCalled(); }); @@ -40,9 +40,9 @@ test('call handler on its event type', () => { test('do not call handler on other event type', () => { const { handler, getEventEmitter, wrapper } = getTestSetup(); - renderHook(() => useKeyboardShortcuts({ 'tab-1': handler }), { wrapper }); + renderHook(() => useKeyboardShortcuts({ tab1: handler }), { wrapper }); const emitEvent = getEventEmitter(); - emitEvent({ type: 'tab-2' }); + emitEvent({ action: 'tab2' }); expect(handler).not.toHaveBeenCalled(); }); diff --git a/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcuts.ts b/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcuts.ts index b944a57b0fd13..94162b4f4eaca 100644 --- a/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcuts.ts +++ b/web/packages/teleterm/src/ui/services/keyboardShortcuts/useKeyboardShortcuts.ts @@ -28,11 +28,11 @@ export function useKeyboardShortcuts(handlers: KeyboardShortcutHandlers): void { useEffect(() => { const handleShortcutEvent: KeyboardShortcutEventSubscriber = event => { - handlers[event.type]?.(); + handlers[event.action]?.(); }; keyboardShortcutsService.subscribeToEvents(handleShortcutEvent); return () => keyboardShortcutsService.unsubscribeFromEvents(handleShortcutEvent); - }, [handlers]); + }, [handlers, keyboardShortcutsService]); } diff --git a/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts b/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts index 502f6429501f3..01c3dc3032c00 100644 --- a/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts +++ b/web/packages/teleterm/src/ui/services/statePersistence/statePersistenceService.ts @@ -106,7 +106,10 @@ export class StatePersistenceService { askedForUserJobRole: false, }, }; - return { ...defaultState, ...this._fileStorage.get('state') }; + return { + ...defaultState, + ...(this._fileStorage.get('state') as StatePersistenceState), + }; } private putState(state: StatePersistenceState): void { diff --git a/web/packages/teleterm/src/ui/services/usage/setUpUsageReporting.ts b/web/packages/teleterm/src/ui/services/usage/setUpUsageReporting.ts index 46f07fdd5f78c..54656f76f8f63 100644 --- a/web/packages/teleterm/src/ui/services/usage/setUpUsageReporting.ts +++ b/web/packages/teleterm/src/ui/services/usage/setUpUsageReporting.ts @@ -29,6 +29,11 @@ export async function setUpUsageReporting( return; } + if (configService.getConfigError()?.source === 'file-loading') { + // do not show the dialog, response cannot be saved to the file + return; + } + if (configService.get('usageReporting.enabled').metadata.isStored) { return; } diff --git a/yarn.lock b/yarn.lock index 9c6869da378f8..a0d9a6daf647c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -15773,10 +15773,15 @@ yocto-queue@^0.1.0: resolved "https://registry.yarnpkg.com/yocto-queue/-/yocto-queue-0.1.0.tgz#0294eb3dee05028d31ee1a5fa2c556a6aaf10a1b" integrity sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q== -zod@^3.20.0: - version "3.20.2" - resolved "https://registry.yarnpkg.com/zod/-/zod-3.20.2.tgz#068606642c8f51b3333981f91c0a8ab37dfc2807" - integrity sha512-1MzNQdAvO+54H+EaK5YpyEy0T+Ejo/7YLHS93G3RnYWh5gaotGHwGeN/ZO687qEDU2y4CdStQYXVHIgrUl5UVQ== +zod-to-json-schema@^3.20.4: + version "3.20.4" + resolved "https://registry.yarnpkg.com/zod-to-json-schema/-/zod-to-json-schema-3.20.4.tgz#155f687c5a059fdc0f1bb3ff32d6e9200036b6f4" + integrity sha512-Un9+kInJ2Zt63n6Z7mLqBifzzPcOyX+b+Exuzf7L1+xqck9Q2EPByyTRduV3kmSPaXaRer1JCsucubpgL1fipg== + +zod@^3.21.2: + version "3.21.2" + resolved "https://registry.yarnpkg.com/zod/-/zod-3.21.2.tgz#a25425916d63b74d5ddd0b2a1bf733ecc093964b" + integrity sha512-0Ygy2/IZNIxHterZdHjE5Vb8hp1fUHJD/BGvSHj8QJx+UipEVNvo9WLchoyBpz5JIaN6KmdGDGYdloGzpFK98g== zone.js@^0.11.0: version "0.11.8"