From 78413c1ecffa781ba654c9440fbcce59024f9b60 Mon Sep 17 00:00:00 2001 From: Michel Weststrate Date: Thu, 16 Dec 2021 14:49:46 -0800 Subject: [PATCH] Register shortcuts on window instead of globally Summary: Changelog: Register shortcuts only for Flipper application instead of globally. Fixes https://github.com/facebook/flipper/issues/3090 As reported, during decapitation we accidentally started registering shortcuts globally, rather per app as it used to be in the menu's. This diff fixes that and also makes sure shortcuts are supported in the browser version of flipper Reviewed By: lawrencelomax Differential Revision: D33158445 fbshipit-source-id: 8371e5b96e772152eeb93ba990e1f5edb9e67085 --- .../app/src/electron/initializeElectron.tsx | 4 --- .../src/initializeRenderHost.tsx | 4 --- desktop/flipper-ui-core/package.json | 1 + desktop/flipper-ui-core/src/RenderHost.tsx | 5 --- .../src/chrome/PluginActionsMenu.tsx | 8 ++--- .../chrome/settings/KeyboardShortcutInput.tsx | 2 +- .../src/dispatcher/reactNative.tsx | 5 ++- .../src/utils/registerShortcut.tsx | 28 +++++++++++++++++ desktop/static/main.ts | 5 --- desktop/yarn.lock | 31 +++++++++++-------- 10 files changed, 52 insertions(+), 41 deletions(-) create mode 100644 desktop/flipper-ui-core/src/utils/registerShortcut.tsx diff --git a/desktop/app/src/electron/initializeElectron.tsx b/desktop/app/src/electron/initializeElectron.tsx index 11a42bb04..bf941aa68 100644 --- a/desktop/app/src/electron/initializeElectron.tsx +++ b/desktop/app/src/electron/initializeElectron.tsx @@ -152,10 +152,6 @@ export function initializeElectron( openLink(url: string) { shell.openExternal(url); }, - registerShortcut(shortcut, callback) { - remote.globalShortcut.register(shortcut, callback); - return () => remote.globalShortcut.unregister(shortcut); - }, hasFocus() { return remote.getCurrentWindow().isFocused(); }, diff --git a/desktop/flipper-ui-browser/src/initializeRenderHost.tsx b/desktop/flipper-ui-browser/src/initializeRenderHost.tsx index 5beed8e99..7f97a9f6a 100644 --- a/desktop/flipper-ui-browser/src/initializeRenderHost.tsx +++ b/desktop/flipper-ui-browser/src/initializeRenderHost.tsx @@ -31,10 +31,6 @@ export function initializeRenderHost( openLink(url: string) { window.open(url, '_blank'); }, - registerShortcut(_shortcut, _callback) { - // TODO: - return () => {}; - }, hasFocus() { return document.hasFocus(); }, diff --git a/desktop/flipper-ui-core/package.json b/desktop/flipper-ui-core/package.json index d07de1374..2416d80ae 100644 --- a/desktop/flipper-ui-core/package.json +++ b/desktop/flipper-ui-core/package.json @@ -32,6 +32,7 @@ "flipper-plugin": "0.0.0", "flipper-ui-core": "0.0.0", "fs-extra": "^10.0.0", + "hotkeys-js": "^3.8.7", "immer": "^9.0.6", "invariant": "^2.2.2", "js-base64": "^3.7.2", diff --git a/desktop/flipper-ui-core/src/RenderHost.tsx b/desktop/flipper-ui-core/src/RenderHost.tsx index c732734c2..08ccad958 100644 --- a/desktop/flipper-ui-core/src/RenderHost.tsx +++ b/desktop/flipper-ui-core/src/RenderHost.tsx @@ -78,11 +78,6 @@ export interface RenderHost { showSelectDirectoryDialog?(defaultPath?: string): Promise; importFile: FlipperLib['importFile']; exportFile: FlipperLib['exportFile']; - /** - * @returns - * A callback to unregister the shortcut - */ - registerShortcut(shortCut: string, callback: () => void): () => void; hasFocus(): boolean; onIpcEvent( event: Event, diff --git a/desktop/flipper-ui-core/src/chrome/PluginActionsMenu.tsx b/desktop/flipper-ui-core/src/chrome/PluginActionsMenu.tsx index eb84d74a7..a57ec40ff 100644 --- a/desktop/flipper-ui-core/src/chrome/PluginActionsMenu.tsx +++ b/desktop/flipper-ui-core/src/chrome/PluginActionsMenu.tsx @@ -17,8 +17,8 @@ import { useTrackedCallback, } from 'flipper-plugin'; import React, {useEffect} from 'react'; -import {getRenderHostInstance} from '../RenderHost'; import {getActivePlugin} from '../selectors/connections'; +import {registerShortcut} from '../utils/registerShortcut'; import {useStore} from '../utils/useStore'; function MagicIcon() { @@ -99,11 +99,7 @@ function PluginActionMenuItem({ useEffect(() => { if (accelerator) { - const unregister = getRenderHostInstance().registerShortcut( - accelerator, - trackedHandler, - ); - return unregister; + return registerShortcut(accelerator, trackedHandler); } }, [trackedHandler, accelerator]); diff --git a/desktop/flipper-ui-core/src/chrome/settings/KeyboardShortcutInput.tsx b/desktop/flipper-ui-core/src/chrome/settings/KeyboardShortcutInput.tsx index ec5034945..2925c96b9 100644 --- a/desktop/flipper-ui-core/src/chrome/settings/KeyboardShortcutInput.tsx +++ b/desktop/flipper-ui-core/src/chrome/settings/KeyboardShortcutInput.tsx @@ -7,7 +7,7 @@ * @format */ -import {FlexColumn, styled, FlexRow, Text, Glyph, colors} from '../../ui'; +import {FlexColumn, styled, FlexRow, Text, Glyph} from '../../ui'; import React, {useRef, useState, useEffect} from 'react'; import {theme} from 'flipper-plugin'; diff --git a/desktop/flipper-ui-core/src/dispatcher/reactNative.tsx b/desktop/flipper-ui-core/src/dispatcher/reactNative.tsx index 4cf223207..d3634f008 100644 --- a/desktop/flipper-ui-core/src/dispatcher/reactNative.tsx +++ b/desktop/flipper-ui-core/src/dispatcher/reactNative.tsx @@ -8,7 +8,7 @@ */ import {Store} from '../reducers'; -import {getRenderHostInstance} from '../RenderHost'; +import {registerShortcut} from '../utils/registerShortcut'; type ShortcutEventCommand = | { @@ -19,7 +19,6 @@ type ShortcutEventCommand = export default (store: Store) => { const settings = store.getState().settingsState.reactNative; - const renderHost = getRenderHostInstance(); if (!settings?.shortcuts.enabled) { return; @@ -40,7 +39,7 @@ export default (store: Store) => { (shortcut: ShortcutEventCommand) => shortcut && shortcut.shortcut && - renderHost.registerShortcut(shortcut.shortcut, () => { + registerShortcut(shortcut.shortcut, () => { const devices = store .getState() .connections.devices.filter( diff --git a/desktop/flipper-ui-core/src/utils/registerShortcut.tsx b/desktop/flipper-ui-core/src/utils/registerShortcut.tsx new file mode 100644 index 000000000..207b2b6c4 --- /dev/null +++ b/desktop/flipper-ui-core/src/utils/registerShortcut.tsx @@ -0,0 +1,28 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @format + */ + +import hotkeys from 'hotkeys-js'; + +export function registerShortcut( + accelerator: string, + handler: () => void, +): () => void { + // normalize between Electron defined shortcuts format, and hotkeys format + // split acceleratos like Shift+CmdOrCtrl+Z into Shift+Cmd+Z,Shift+Control+Z + if (accelerator.includes('CmdOrCtrl')) { + accelerator = + accelerator.replace('CmdOrCtrl', 'Cmd') + + ',' + + accelerator.replace('CmdOrCtrl', 'Ctrl'); + } + hotkeys(accelerator, handler); + return () => { + hotkeys.unbind(accelerator, handler); + }; +} diff --git a/desktop/static/main.ts b/desktop/static/main.ts index 0badb7021..9cf86429f 100644 --- a/desktop/static/main.ts +++ b/desktop/static/main.ts @@ -16,7 +16,6 @@ import { BrowserWindow, ipcMain, Notification, - globalShortcut, session, nativeTheme, shell, @@ -261,10 +260,6 @@ function configureSession() { ); } -app.on('will-quit', () => { - globalShortcut.unregisterAll(); -}); - ipcMain.on('componentDidMount', (_event) => { didMount = true; if (deeplinkURL) { diff --git a/desktop/yarn.lock b/desktop/yarn.lock index bda46273a..34ac3a0e3 100644 --- a/desktop/yarn.lock +++ b/desktop/yarn.lock @@ -4332,12 +4332,12 @@ browser-resolve@^1.11.3: resolve "1.1.7" browserslist@^4.16.5, browserslist@^4.17.5, browserslist@^4.17.6: - version "4.18.1" - resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.18.1.tgz#60d3920f25b6860eb917c6c7b185576f4d8b017f" - integrity sha512-8ScCzdpPwR2wQh8IT82CA2VgDwjHyqMovPBZSNH54+tm4Jk2pCuv90gmAdH6J84OCRWi0b4gMe6O6XPXuJnjgQ== + version "4.19.1" + resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.19.1.tgz#4ac0435b35ab655896c31d53018b6dd5e9e4c9a3" + integrity sha512-u2tbbG5PdKRTUoctO3NBD8FQ5HdPh1ZXPHzp1rwaa5jTc+RV9/+RlWiAIKmjRPQF+xbGM9Kklj5bZQFa2s/38A== dependencies: - caniuse-lite "^1.0.30001280" - electron-to-chromium "^1.3.896" + caniuse-lite "^1.0.30001286" + electron-to-chromium "^1.4.17" escalade "^3.1.1" node-releases "^2.0.1" picocolors "^1.0.0" @@ -4547,10 +4547,10 @@ camelcase@^6.0.0, camelcase@^6.2.0: resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-6.2.0.tgz#924af881c9d525ac9d87f40d964e5cea982a1809" integrity sha512-c7wVvbw3f37nuobQNtgsgG9POC9qMbNuMQmTCqZv23b6MIz0fcYpBiOlv9gEN/hdLdnZTDQhg6e9Dq5M1vKvfg== -caniuse-lite@^1.0.30001280: - version "1.0.30001280" - resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001280.tgz#066a506046ba4be34cde5f74a08db7a396718fb7" - integrity sha512-kFXwYvHe5rix25uwueBxC569o53J6TpnGu0BEEn+6Lhl2vsnAumRFWEBhDft1fwyo6m1r4i+RqA4+163FpeFcA== +caniuse-lite@^1.0.30001286: + version "1.0.30001287" + resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001287.tgz#5fab6a46ab9e47146d5dd35abfe47beaf8073c71" + integrity sha512-4udbs9bc0hfNrcje++AxBuc6PfLNHwh3PO9kbwnfCQWyqtlzg3py0YgFu8jyRTTo85VAz4U+VLxSlID09vNtWA== capture-exit@^2.0.0: version "2.0.0" @@ -5739,10 +5739,10 @@ electron-publish@22.14.7: lazy-val "^1.0.5" mime "^2.5.2" -electron-to-chromium@^1.3.896: - version "1.3.896" - resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.896.tgz#4a94efe4870b1687eafd5c378198a49da06e8a1b" - integrity sha512-NcGkBVXePiuUrPLV8IxP43n1EOtdg+dudVjrfVEUd/bOqpQUFZ2diL5PPYzbgEhZFEltdXV3AcyKwGnEQ5lhMA== +electron-to-chromium@^1.4.17: + version "1.4.20" + resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.4.20.tgz#8fbf9677ccac19b4249c0a6204e0943d9d66ce30" + integrity sha512-N7ZVNrdzX8NE90OXEFBMsBf3fp8P/vVDUER3WCUZjzC7OkNTXHVoF6W9qVhq8+dA8tGnbDajzUpj2ISNVVyj+Q== electron@11.2.3: version "11.2.3" @@ -7242,6 +7242,11 @@ hosted-git-info@^4.0.2: dependencies: lru-cache "^6.0.0" +hotkeys-js@^3.8.7: + version "3.8.7" + resolved "https://registry.yarnpkg.com/hotkeys-js/-/hotkeys-js-3.8.7.tgz#c16cab978b53d7242f860ca3932e976b92399981" + integrity sha512-ckAx3EkUr5XjDwjEHDorHxRO2Kb7z6Z2Sxul4MbBkN8Nho7XDslQsgMJT+CiJ5Z4TgRxxvKHEpuLE3imzqy4Lg== + html-encoding-sniffer@^2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/html-encoding-sniffer/-/html-encoding-sniffer-2.0.1.tgz#42a6dc4fd33f00281176e8b23759ca4e4fa185f3"