From 24e42daa5142b5d494500c17e6349006f2036e00 Mon Sep 17 00:00:00 2001
From: Matiss Janis Aboltins <matiss@mja.lv>
Date: Sun, 24 Mar 2024 07:08:07 +0000
Subject: [PATCH] :bug: fix hotkeys sometimes not working (#2489)
---
.eslintrc.js | 1 -
packages/desktop-client/package.json | 2 +-
.../desktop-client/src/components/App.tsx | 41 ++++----
.../src/components/FinancesApp.tsx | 5 -
.../src/components/KeyHandlers.tsx | 87 ----------------
.../src/components/Titlebar.tsx | 99 ++++++++++---------
.../src/components/accounts/Header.jsx | 27 ++---
.../src/components/common/Modal.tsx | 17 ++--
.../desktop-client/src/components/table.tsx | 5 +-
.../transactions/SelectedTransactions.jsx | 45 ++++++---
upcoming-release-notes/2489.md | 6 ++
yarn.lock | 19 ++--
12 files changed, 149 insertions(+), 205 deletions(-)
delete mode 100644 packages/desktop-client/src/components/KeyHandlers.tsx
create mode 100644 upcoming-release-notes/2489.md
diff --git a/.eslintrc.js b/.eslintrc.js
index a1374f06b..6762550f2 100644
--- a/.eslintrc.js
+++ b/.eslintrc.js
@@ -287,7 +287,6 @@ module.exports = {
'./packages/desktop-client/src/components/common/Menu.tsx',
'./packages/desktop-client/src/components/FinancesApp.tsx',
'./packages/desktop-client/src/components/GlobalKeys.ts',
- './packages/desktop-client/src/components/KeyHandlers.tsx',
'./packages/desktop-client/src/components/LoggedInUser.tsx',
'./packages/desktop-client/src/components/manager/ManagementApp.jsx',
'./packages/desktop-client/src/components/manager/subscribe/common.tsx',
diff --git a/packages/desktop-client/package.json b/packages/desktop-client/package.json
index 9e4d28668..a5a2f5592 100644
--- a/packages/desktop-client/package.json
+++ b/packages/desktop-client/package.json
@@ -37,7 +37,6 @@
"downshift": "7.6.2",
"focus-visible": "^4.1.5",
"glamor": "^2.20.40",
- "hotkeys-js": "^3.13.5",
"inter-ui": "^3.19.3",
"jest": "^27.5.1",
"jest-watch-typeahead": "^2.2.2",
@@ -49,6 +48,7 @@
"react-dnd-html5-backend": "^16.0.1",
"react-dom": "18.2.0",
"react-error-boundary": "^4.0.12",
+ "react-hotkeys-hook": "^4.5.0",
"react-markdown": "^8.0.7",
"react-merge-refs": "^1.1.0",
"react-modal": "3.16.1",
diff --git a/packages/desktop-client/src/components/App.tsx b/packages/desktop-client/src/components/App.tsx
index 8323f7247..0b4ea838e 100644
--- a/packages/desktop-client/src/components/App.tsx
+++ b/packages/desktop-client/src/components/App.tsx
@@ -5,6 +5,7 @@ import {
useErrorBoundary,
type FallbackProps,
} from 'react-error-boundary';
+import { HotkeysProvider } from 'react-hotkeys-hook';
import { useSelector } from 'react-redux';
import * as Platform from 'loot-core/src/client/platform';
@@ -146,27 +147,29 @@ export function App() {
}, [sync]);
return (
- <ResponsiveProvider>
- <View
- style={{ height: '100%', display: 'flex', flexDirection: 'column' }}
- >
+ <HotkeysProvider initiallyActiveScopes={['*']}>
+ <ResponsiveProvider>
<View
- key={hiddenScrollbars ? 'hidden-scrollbars' : 'scrollbars'}
- style={{
- flexGrow: 1,
- overflow: 'hidden',
- ...styles.lightScrollbar,
- }}
+ style={{ height: '100%', display: 'flex', flexDirection: 'column' }}
>
- <ErrorBoundary FallbackComponent={ErrorFallback}>
- {process.env.REACT_APP_REVIEW_ID && !Platform.isPlaywright && (
- <DevelopmentTopBar />
- )}
- <AppInner budgetId={budgetId} cloudFileId={cloudFileId} />
- </ErrorBoundary>
- <ThemeStyle />
+ <View
+ key={hiddenScrollbars ? 'hidden-scrollbars' : 'scrollbars'}
+ style={{
+ flexGrow: 1,
+ overflow: 'hidden',
+ ...styles.lightScrollbar,
+ }}
+ >
+ <ErrorBoundary FallbackComponent={ErrorFallback}>
+ {process.env.REACT_APP_REVIEW_ID && !Platform.isPlaywright && (
+ <DevelopmentTopBar />
+ )}
+ <AppInner budgetId={budgetId} cloudFileId={cloudFileId} />
+ </ErrorBoundary>
+ <ThemeStyle />
+ </View>
</View>
- </View>
- </ResponsiveProvider>
+ </ResponsiveProvider>
+ </HotkeysProvider>
);
}
diff --git a/packages/desktop-client/src/components/FinancesApp.tsx b/packages/desktop-client/src/components/FinancesApp.tsx
index 1dbf53dac..2a1bc50a0 100644
--- a/packages/desktop-client/src/components/FinancesApp.tsx
+++ b/packages/desktop-client/src/components/FinancesApp.tsx
@@ -12,8 +12,6 @@ import {
useHref,
} from 'react-router-dom';
-import hotkeys from 'hotkeys-js';
-
import { SpreadsheetProvider } from 'loot-core/src/client/SpreadsheetProvider';
import { type State } from 'loot-core/src/client/state-types';
import { checkForUpdateNotification } from 'loot-core/src/client/update-notification';
@@ -99,9 +97,6 @@ function RouterBehaviors() {
function FinancesAppWithoutContext() {
const actions = useActions();
useEffect(() => {
- // The default key handler scope
- hotkeys.setScope('app');
-
// Wait a little bit to make sure the sync button will get the
// sync start event. This can be improved later.
setTimeout(async () => {
diff --git a/packages/desktop-client/src/components/KeyHandlers.tsx b/packages/desktop-client/src/components/KeyHandlers.tsx
deleted file mode 100644
index 13affa339..000000000
--- a/packages/desktop-client/src/components/KeyHandlers.tsx
+++ /dev/null
@@ -1,87 +0,0 @@
-// @ts-strict-ignore
-import React, { createContext, useEffect, useContext } from 'react';
-
-import hotkeys, { type KeyHandler as HotKeyHandler } from 'hotkeys-js';
-
-const KeyScopeContext = createContext('app');
-
-hotkeys.filter = event => {
- const target = (event.target || event.srcElement) as HTMLElement;
- const tagName = target.tagName;
-
- // This is the default behavior of hotkeys, except we only suppress
- // key presses if the meta key is not pressed
- if (
- !event.metaKey &&
- (target.isContentEditable ||
- ((tagName === 'INPUT' ||
- tagName === 'TEXTAREA' ||
- tagName === 'SELECT') &&
- !target['readOnly']))
- ) {
- return false;
- }
-
- return true;
-};
-
-type KeyHandlerProps = {
- keyName: string;
- eventType?: string;
- handler: HotKeyHandler;
-};
-function KeyHandler({
- keyName,
- eventType = 'keydown',
- handler,
-}: KeyHandlerProps) {
- const scope = useContext(KeyScopeContext);
-
- if (eventType !== 'keyup' && eventType !== 'keydown') {
- throw new Error('KeyHandler: unknown event type: ' + eventType);
- }
-
- useEffect(() => {
- function _handler(event, hk) {
- // Right now it always overrides the default behavior, but in
- // the future we can make this customizable
- event.preventDefault();
-
- if (event.type === eventType && handler) {
- return handler(event, hk);
- }
- }
- hotkeys(keyName, { scope, keyup: true }, _handler);
-
- return () => {
- // @ts-expect-error unbind args typedef does not expect an object
- hotkeys.unbind({
- key: keyName,
- scope,
- method: _handler,
- });
- };
- }, [keyName, handler, scope]);
-
- return null;
-}
-
-type KeyHandlersProps = {
- eventType?: string;
- keys: Record<string, HotKeyHandler>;
-};
-export function KeyHandlers({ eventType, keys = {} }: KeyHandlersProps) {
- const handlers = Object.keys(keys).map(key => {
- return (
- <KeyHandler
- key={key}
- keyName={key}
- eventType={eventType}
- handler={keys[key]}
- />
- );
- });
-
- // eslint-disable-next-line react/jsx-no-useless-fragment
- return <>{handlers}</>;
-}
diff --git a/packages/desktop-client/src/components/Titlebar.tsx b/packages/desktop-client/src/components/Titlebar.tsx
index 8a08b283f..ad7e20ef6 100644
--- a/packages/desktop-client/src/components/Titlebar.tsx
+++ b/packages/desktop-client/src/components/Titlebar.tsx
@@ -6,6 +6,7 @@ import React, {
useContext,
type ReactNode,
} from 'react';
+import { useHotkeys } from 'react-hotkeys-hook';
import { Routes, Route, useLocation } from 'react-router-dom';
import * as Platform from 'loot-core/src/client/platform';
@@ -37,7 +38,6 @@ import { Link } from './common/Link';
import { Paragraph } from './common/Paragraph';
import { Text } from './common/Text';
import { View } from './common/View';
-import { KeyHandlers } from './KeyHandlers';
import { LoggedInUser } from './LoggedInUser';
import { useServerURL } from './ServerContext';
import { useSidebar } from './sidebar/SidebarProvider';
@@ -231,56 +231,57 @@ function SyncButton({ style, isMobile = false }: SyncButtonProps) {
marginRight: 5,
};
- return (
- <>
- <KeyHandlers
- keys={{
- 'ctrl+s, cmd+s': () => {
- sync();
- },
- }}
- />
+ useHotkeys(
+ 'ctrl+s, cmd+s, meta+s',
+ sync,
+ {
+ enableOnFormTags: true,
+ preventDefault: true,
+ scopes: ['app'],
+ },
+ [sync],
+ );
- <Button
- type="bare"
- aria-label="Sync"
- style={
- isMobile
- ? {
- ...style,
- WebkitAppRegion: 'none',
- ...mobileIconStyle,
- }
- : {
- ...style,
- WebkitAppRegion: 'none',
- color: desktopColor,
- }
- }
- hoveredStyle={hoveredStyle}
- activeStyle={activeStyle}
- onClick={sync}
- >
- {isMobile ? (
- syncState === 'error' ? (
- <SvgAlertTriangle width={14} height={14} />
- ) : (
- <AnimatedRefresh width={18} height={18} animating={syncing} />
- )
- ) : syncState === 'error' ? (
- <SvgAlertTriangle width={13} />
+ return (
+ <Button
+ type="bare"
+ aria-label="Sync"
+ style={
+ isMobile
+ ? {
+ ...style,
+ WebkitAppRegion: 'none',
+ ...mobileIconStyle,
+ }
+ : {
+ ...style,
+ WebkitAppRegion: 'none',
+ color: desktopColor,
+ }
+ }
+ hoveredStyle={hoveredStyle}
+ activeStyle={activeStyle}
+ onClick={sync}
+ >
+ {isMobile ? (
+ syncState === 'error' ? (
+ <SvgAlertTriangle width={14} height={14} />
) : (
- <AnimatedRefresh animating={syncing} />
- )}
- <Text style={isMobile ? { ...mobileTextStyle } : { marginLeft: 3 }}>
- {syncState === 'disabled'
- ? 'Disabled'
- : syncState === 'offline'
- ? 'Offline'
- : 'Sync'}
- </Text>
- </Button>
- </>
+ <AnimatedRefresh width={18} height={18} animating={syncing} />
+ )
+ ) : syncState === 'error' ? (
+ <SvgAlertTriangle width={13} />
+ ) : (
+ <AnimatedRefresh animating={syncing} />
+ )}
+ <Text style={isMobile ? { ...mobileTextStyle } : { marginLeft: 3 }}>
+ {syncState === 'disabled'
+ ? 'Disabled'
+ : syncState === 'offline'
+ ? 'Offline'
+ : 'Sync'}
+ </Text>
+ </Button>
);
}
diff --git a/packages/desktop-client/src/components/accounts/Header.jsx b/packages/desktop-client/src/components/accounts/Header.jsx
index 078203fa6..1dea67b9f 100644
--- a/packages/desktop-client/src/components/accounts/Header.jsx
+++ b/packages/desktop-client/src/components/accounts/Header.jsx
@@ -1,4 +1,5 @@
import React, { useState, useRef } from 'react';
+import { useHotkeys } from 'react-hotkeys-hook';
import { useLocalPref } from '../../hooks/useLocalPref';
import { useSplitsExpanded } from '../../hooks/useSplitsExpanded';
@@ -24,7 +25,6 @@ import { Stack } from '../common/Stack';
import { View } from '../common/View';
import { FilterButton } from '../filters/FiltersMenu';
import { FiltersStack } from '../filters/FiltersStack';
-import { KeyHandlers } from '../KeyHandlers';
import { NotesButton } from '../NotesButton';
import { SelectedTransactionsButton } from '../transactions/SelectedTransactions';
@@ -109,18 +109,23 @@ export function AccountHeader({
}
}
+ useHotkeys(
+ 'ctrl+f, cmd+f, meta+f',
+ () => {
+ if (searchInput.current) {
+ searchInput.current.focus();
+ }
+ },
+ {
+ enableOnFormTags: true,
+ preventDefault: true,
+ scopes: ['app'],
+ },
+ [searchInput],
+ );
+
return (
<>
- <KeyHandlers
- keys={{
- 'ctrl+f, cmd+f': () => {
- if (searchInput.current) {
- searchInput.current.focus();
- }
- },
- }}
- />
-
<View style={{ ...styles.pageContent, paddingBottom: 10, flexShrink: 0 }}>
<View
style={{ marginTop: 2, marginBottom: 10, alignItems: 'flex-start' }}
diff --git a/packages/desktop-client/src/components/common/Modal.tsx b/packages/desktop-client/src/components/common/Modal.tsx
index 13a7e3208..9c07af48f 100644
--- a/packages/desktop-client/src/components/common/Modal.tsx
+++ b/packages/desktop-client/src/components/common/Modal.tsx
@@ -6,10 +6,9 @@ import React, {
type ReactNode,
useState,
} from 'react';
+import { useHotkeysContext } from 'react-hotkeys-hook';
import ReactModal from 'react-modal';
-import hotkeys from 'hotkeys-js';
-
import { AnimatedLoading } from '../../icons/AnimatedLoading';
import { SvgDelete } from '../../icons/v0';
import { type CSSProperties, styles, theme } from '../../style';
@@ -75,14 +74,14 @@ export const Modal = ({
onClose,
onTitleUpdate,
}: ModalProps) => {
+ const { enableScope, disableScope } = useHotkeysContext();
+
+ // This deactivates any key handlers in the "app" scope
+ const scopeId = `modal-${stackIndex}-${title}`;
useEffect(() => {
- // This deactivates any key handlers in the "app" scope. Ideally
- // each modal would have a name so they could each have their own
- // key handlers, but we'll do that later
- const prevScope = hotkeys.getScope();
- hotkeys.setScope('modal');
- return () => hotkeys.setScope(prevScope);
- }, []);
+ enableScope(scopeId);
+ return () => disableScope(scopeId);
+ }, [enableScope, disableScope, scopeId]);
const [isEditingTitle, setIsEditingTitle] = useState(false);
const [_title, setTitle] = useState(title);
diff --git a/packages/desktop-client/src/components/table.tsx b/packages/desktop-client/src/components/table.tsx
index 1259d0865..59ff2f195 100644
--- a/packages/desktop-client/src/components/table.tsx
+++ b/packages/desktop-client/src/components/table.tsx
@@ -34,7 +34,6 @@ import { Menu } from './common/Menu';
import { Text } from './common/Text';
import { View } from './common/View';
import { FixedSizeList } from './FixedSizeList';
-import { KeyHandlers } from './KeyHandlers';
import {
ConditionalPrivacyFilter,
mergeConditionalPrivacyFilterProps,
@@ -785,7 +784,7 @@ export function TableHeader({
);
}
-export function SelectedItemsButton({ name, keyHandlers, items, onSelect }) {
+export function SelectedItemsButton({ name, items, onSelect }) {
const selectedItems = useSelectedItems();
const [menuOpen, setMenuOpen] = useState(null);
@@ -795,8 +794,6 @@ export function SelectedItemsButton({ name, keyHandlers, items, onSelect }) {
return (
<View style={{ marginLeft: 10, flexShrink: 0 }}>
- <KeyHandlers keys={keyHandlers || {}} />
-
<Button
type="bare"
style={{ color: theme.pageTextPositive }}
diff --git a/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx b/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
index dad536a1d..d08e73c70 100644
--- a/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
+++ b/packages/desktop-client/src/components/transactions/SelectedTransactions.jsx
@@ -1,4 +1,5 @@
import React, { useMemo } from 'react';
+import { useHotkeys } from 'react-hotkeys-hook';
import { isPreviewId } from 'loot-core/shared/transactions';
import { validForTransfer } from 'loot-core/src/client/transfer';
@@ -63,20 +64,42 @@ export function SelectedTransactionsButton({
return validForTransfer(fromTrans, toTrans);
}, [selectedItems, getTransaction]);
+ const hotKeyOptions = {
+ enabled: types.trans,
+ scopes: ['app'],
+ };
+ useHotkeys('f', () => onShow([...selectedItems]), hotKeyOptions, [
+ onShow,
+ selectedItems,
+ ]);
+ useHotkeys('d', () => onDelete([...selectedItems]), hotKeyOptions, [
+ onDelete,
+ selectedItems,
+ ]);
+ useHotkeys('a', () => onEdit('account', [...selectedItems]), hotKeyOptions, [
+ onEdit,
+ selectedItems,
+ ]);
+ useHotkeys('p', () => onEdit('payee', [...selectedItems]), hotKeyOptions, [
+ onEdit,
+ selectedItems,
+ ]);
+ useHotkeys('n', () => onEdit('notes', [...selectedItems]), hotKeyOptions, [
+ onEdit,
+ selectedItems,
+ ]);
+ useHotkeys('c', () => onEdit('category', [...selectedItems]), hotKeyOptions, [
+ onEdit,
+ selectedItems,
+ ]);
+ useHotkeys('l', () => onEdit('cleared', [...selectedItems]), hotKeyOptions, [
+ onEdit,
+ selectedItems,
+ ]);
+
return (
<SelectedItemsButton
name="transactions"
- keyHandlers={
- types.trans && {
- f: () => onShow([...selectedItems]),
- d: () => onDelete([...selectedItems]),
- a: () => onEdit('account', [...selectedItems]),
- p: () => onEdit('payee', [...selectedItems]),
- n: () => onEdit('notes', [...selectedItems]),
- c: () => onEdit('category', [...selectedItems]),
- l: () => onEdit('cleared', [...selectedItems]),
- }
- }
items={[
...(!types.trans
? [
diff --git a/upcoming-release-notes/2489.md b/upcoming-release-notes/2489.md
new file mode 100644
index 000000000..1d0bcba0d
--- /dev/null
+++ b/upcoming-release-notes/2489.md
@@ -0,0 +1,6 @@
+---
+category: Bugfix
+authors: [MatissJanis]
+---
+
+Fix hotkeys sometimes stopping to work.
diff --git a/yarn.lock b/yarn.lock
index 53cfcdcb7..c000e24ac 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -89,7 +89,6 @@ __metadata:
downshift: "npm:7.6.2"
focus-visible: "npm:^4.1.5"
glamor: "npm:^2.20.40"
- hotkeys-js: "npm:^3.13.5"
inter-ui: "npm:^3.19.3"
jest: "npm:^27.5.1"
jest-watch-typeahead: "npm:^2.2.2"
@@ -101,6 +100,7 @@ __metadata:
react-dnd-html5-backend: "npm:^16.0.1"
react-dom: "npm:18.2.0"
react-error-boundary: "npm:^4.0.12"
+ react-hotkeys-hook: "npm:^4.5.0"
react-markdown: "npm:^8.0.7"
react-merge-refs: "npm:^1.1.0"
react-modal: "npm:3.16.1"
@@ -10534,13 +10534,6 @@ __metadata:
languageName: node
linkType: hard
-"hotkeys-js@npm:^3.13.5":
- version: 3.13.5
- resolution: "hotkeys-js@npm:3.13.5"
- checksum: 739ac5924cfdd67571bfba85c0e6c0bc8ea1d617247b0e7126b1c4a3d04d14dbd35f5ee8455ac83f8703195b00c31eb8d8c06489143338fff9ac4129d16c007f
- languageName: node
- linkType: hard
-
"html-encoding-sniffer@npm:^2.0.1":
version: 2.0.1
resolution: "html-encoding-sniffer@npm:2.0.1"
@@ -15070,6 +15063,16 @@ __metadata:
languageName: node
linkType: hard
+"react-hotkeys-hook@npm:^4.5.0":
+ version: 4.5.0
+ resolution: "react-hotkeys-hook@npm:4.5.0"
+ peerDependencies:
+ react: ">=16.8.1"
+ react-dom: ">=16.8.1"
+ checksum: f83d4ef7d56e40717f293d936101fb9d72f83b92568d5eb85594cf99e2b7c66556ff8f94e6dd9008813c7d097185593ff1b716b941ccf7ebf1969fecb4dc4b84
+ languageName: node
+ linkType: hard
+
"react-is@npm:^16.10.2, react-is@npm:^16.13.1, react-is@npm:^16.7.0":
version: 16.13.1
resolution: "react-is@npm:16.13.1"
--
GitLab