From b638f4d5f2d582da9f822faaf54dddea0036a328 Mon Sep 17 00:00:00 2001 From: trevor-signal <131492920+trevor-signal@users.noreply.github.com> Date: Tue, 9 Dec 2025 10:53:43 -0500 Subject: [PATCH] Improve reliability of keyboard shortcuts for composer --- ts/components/CompositionArea.dom.tsx | 20 ++-- ts/components/CompositionInput.dom.tsx | 98 +++++++++++-------- .../conversation/TimelineMessage.dom.tsx | 15 ++- ts/hooks/useDocumentKeyDown.dom.ts | 51 ++++++++++ ts/hooks/useKeyboardShortcuts.dom.tsx | 20 ---- ts/util/lint/exceptions.json | 7 ++ 6 files changed, 128 insertions(+), 83 deletions(-) create mode 100644 ts/hooks/useDocumentKeyDown.dom.ts diff --git a/ts/components/CompositionArea.dom.tsx b/ts/components/CompositionArea.dom.tsx index 81d3a35603..e71209a9c4 100644 --- a/ts/components/CompositionArea.dom.tsx +++ b/ts/components/CompositionArea.dom.tsx @@ -52,7 +52,6 @@ import { Quote } from './conversation/Quote.dom.js'; import { useAttachFileShortcut, useEditLastMessageSent, - useKeyboardShortcutsConditionally, } from '../hooks/useKeyboardShortcuts.dom.js'; import { MediaEditor } from './MediaEditor.dom.js'; import { isImageTypeSupported } from '../util/GoogleChrome.std.js'; @@ -82,6 +81,7 @@ import { AxoButton } from '../axo/AxoButton.dom.js'; import { tw } from '../axo/tw.dom.js'; import { isPollSendEnabled, type PollCreateType } from '../types/Polls.dom.js'; import { PollCreateModal } from './PollCreateModal.dom.js'; +import { useDocumentKeyDown } from '../hooks/useDocumentKeyDown.dom.js'; export type OwnProps = Readonly<{ acceptedMessageRequest: boolean | null; @@ -479,15 +479,15 @@ export const CompositionArea = memo(function CompositionArea({ setMessageToEdit, ]); - const [hasFocus, setHasFocus] = useState(false); - const attachFileShortcut = useAttachFileShortcut(launchFilePicker); const editLastMessageSent = useEditLastMessageSent(maybeEditMessage); - useKeyboardShortcutsConditionally( - hasFocus, - attachFileShortcut, - editLastMessageSent - ); + useDocumentKeyDown(event => { + const hasFocus = inputApiRef.current?.hasFocus() ?? false; + if (hasFocus) { + attachFileShortcut(event); + editLastMessageSent(event); + } + }); // Focus input on first mount const previousFocusCounter = usePrevious( @@ -497,14 +497,12 @@ export const CompositionArea = memo(function CompositionArea({ useEffect(() => { if (inputApiRef.current) { inputApiRef.current.focus(); - setHasFocus(true); } }, []); // Focus input whenever explicitly requested useEffect(() => { if (focusCounter !== previousFocusCounter && inputApiRef.current) { inputApiRef.current.focus(); - setHasFocus(true); } }, [inputApiRef, focusCounter, previousFocusCounter]); @@ -1162,8 +1160,6 @@ export const CompositionArea = memo(function CompositionArea({ large={large} linkPreviewLoading={linkPreviewLoading} linkPreviewResult={linkPreviewResult} - onBlur={() => setHasFocus(false)} - onFocus={() => setHasFocus(true)} onCloseLinkPreview={onCloseLinkPreview} onDirtyChange={setDirty} onEditorStateChange={onEditorStateChange} diff --git a/ts/components/CompositionInput.dom.tsx b/ts/components/CompositionInput.dom.tsx index a575c23780..24c69b4c2a 100644 --- a/ts/components/CompositionInput.dom.tsx +++ b/ts/components/CompositionInput.dom.tsx @@ -104,6 +104,7 @@ Quill.register( export type InputApi = { focus: () => void; + hasFocus: () => boolean; insertEmoji: (emojiSelection: FunEmojiSelection) => void; setContents: ( text: string, @@ -273,7 +274,7 @@ export function CompositionInput(props: Props): React.ReactElement { }; }; - const focus = () => { + const focus = React.useCallback(() => { const quill = quillRef.current; if (quill === undefined) { @@ -281,34 +282,37 @@ export function CompositionInput(props: Props): React.ReactElement { } quill.focus(); - }; + }, []); - const insertEmoji = (emojiSelection: FunEmojiSelection) => { - const quill = quillRef.current; + const insertEmoji = React.useCallback( + (emojiSelection: FunEmojiSelection) => { + const quill = quillRef.current; - if (quill === undefined) { - return; - } + if (quill === undefined) { + return; + } - const range = quill.getSelection(); + const range = quill.getSelection(); - const insertionRange = range || lastSelectionRange; - if (insertionRange == null) { - return; - } + const insertionRange = range || lastSelectionRange; + if (insertionRange == null) { + return; + } - const emojiVariant = getEmojiVariantByKey(emojiSelection.variantKey); + const emojiVariant = getEmojiVariantByKey(emojiSelection.variantKey); - const delta = new Delta() - .retain(insertionRange.index) - .delete(insertionRange.length) - .insert({ emoji: { value: emojiVariant.value } }); + const delta = new Delta() + .retain(insertionRange.index) + .delete(insertionRange.length) + .insert({ emoji: { value: emojiVariant.value } }); - quill.updateContents(delta, 'user'); - quill.setSelection(insertionRange.index + 1, 0, 'user'); - }; + quill.updateContents(delta, 'user'); + quill.setSelection(insertionRange.index + 1, 0, 'user'); + }, + [lastSelectionRange] + ); - const reset = () => { + const reset = React.useCallback(() => { const quill = quillRef.current; if (quill === undefined) { @@ -319,29 +323,32 @@ export function CompositionInput(props: Props): React.ReactElement { quill.setText(''); quill.history.clear(); - }; + }, []); - const setContents = ( - text: string, - bodyRanges?: HydratedBodyRangesType, - cursorToEnd?: boolean - ) => { - const quill = quillRef.current; + const setContents = React.useCallback( + ( + text: string, + bodyRanges?: HydratedBodyRangesType, + cursorToEnd?: boolean + ) => { + const quill = quillRef.current; - if (quill === undefined) { - return; - } + if (quill === undefined) { + return; + } - const delta = generateDelta(text || '', bodyRanges || []); + const delta = generateDelta(text || '', bodyRanges || []); - canSendRef.current = true; - quill.setContents(delta); - if (cursorToEnd) { - quill.setSelection(quill.getLength(), 0); - } - }; + canSendRef.current = true; + quill.setContents(delta); + if (cursorToEnd) { + quill.setSelection(quill.getLength(), 0); + } + }, + [] + ); - const submit = () => { + const submit = React.useCallback(() => { const timestamp = Date.now(); const quill = quillRef.current; @@ -365,17 +372,22 @@ export function CompositionInput(props: Props): React.ReactElement { if (!didSend) { canSendRef.current = true; } - }; + }, [onSubmit]); - if (inputApi) { - inputApi.current = { + const hasFocus = React.useCallback(() => { + return quillRef.current?.hasFocus() ?? false; + }, []); + + React.useImperativeHandle(inputApi, () => { + return { focus, + hasFocus, insertEmoji, setContents, reset, submit, }; - } + }, [focus, hasFocus, insertEmoji, reset, setContents, submit]); React.useEffect(() => { propsRef.current = props; diff --git a/ts/components/conversation/TimelineMessage.dom.tsx b/ts/components/conversation/TimelineMessage.dom.tsx index c8a963bfca..2c1dfc992d 100644 --- a/ts/components/conversation/TimelineMessage.dom.tsx +++ b/ts/components/conversation/TimelineMessage.dom.tsx @@ -23,10 +23,7 @@ import type { } from './Message.dom.js'; import type { PushPanelForConversationActionType } from '../../state/ducks/conversations.preload.js'; import { doesMessageBodyOverflow } from './MessageBodyReadMore.dom.js'; -import { - useKeyboardShortcutsConditionally, - useToggleReactionPicker, -} from '../../hooks/useKeyboardShortcuts.dom.js'; +import { useToggleReactionPicker } from '../../hooks/useKeyboardShortcuts.dom.js'; import { PanelType } from '../../types/Panels.std.js'; import type { DeleteMessagesPropsType, @@ -40,6 +37,7 @@ import { isNotNil } from '../../util/isNotNil.std.js'; import type { AxoMenuBuilder } from '../../axo/AxoMenuBuilder.dom.js'; import { AxoContextMenu } from '../../axo/AxoContextMenu.dom.js'; import { PinMessageDialog } from './pinned-messages/PinMessageDialog.dom.js'; +import { useDocumentKeyDown } from '../../hooks/useDocumentKeyDown.dom.js'; const { useAxoContextMenuOutsideKeyboardTrigger } = AxoContextMenu; @@ -283,10 +281,11 @@ export function TimelineMessage(props: Props): JSX.Element { handleReact || noop ); - useKeyboardShortcutsConditionally( - Boolean(isTargeted), - toggleReactionPickerKeyboard - ); + useDocumentKeyDown(event => { + if (isTargeted) { + toggleReactionPickerKeyboard(event); + } + }); const groupedReactions = useGroupedAndOrderedReactions( props.reactions, diff --git a/ts/hooks/useDocumentKeyDown.dom.ts b/ts/hooks/useDocumentKeyDown.dom.ts new file mode 100644 index 0000000000..8c36dc5f77 --- /dev/null +++ b/ts/hooks/useDocumentKeyDown.dom.ts @@ -0,0 +1,51 @@ +// Copyright 2025 Signal Messenger, LLC +// SPDX-License-Identifier: AGPL-3.0-only + +import { useEffect, useRef } from 'react'; + +type KeyDownHandler = (e: KeyboardEvent) => void; +const handlers = new Set(); + +let isListenerAttached = false; + +function onKeyDown(event: KeyboardEvent) { + for (const handler of handlers) { + handler(event); + } +} + +function addKeyDownHandler(handler: KeyDownHandler) { + handlers.add(handler); + + if (!isListenerAttached) { + document.addEventListener('keydown', onKeyDown); + isListenerAttached = true; + } +} + +function removeKeyDownCallback(handler: KeyDownHandler) { + handlers.delete(handler); + + if (isListenerAttached && handlers.size === 0) { + document.removeEventListener('keydown', onKeyDown); + isListenerAttached = false; + } +} + +export function useDocumentKeyDown( + listener: (event: KeyboardEvent) => void +): void { + const listenerRef = useRef(listener); + useEffect(() => { + listenerRef.current = listener; + }, [listener]); + + useEffect(() => { + function handler(event: KeyboardEvent) { + listenerRef.current(event); + } + + addKeyDownHandler(handler); + return () => removeKeyDownCallback(handler); + }, []); +} diff --git a/ts/hooks/useKeyboardShortcuts.dom.tsx b/ts/hooks/useKeyboardShortcuts.dom.tsx index b426930629..66d6dac1d5 100644 --- a/ts/hooks/useKeyboardShortcuts.dom.tsx +++ b/ts/hooks/useKeyboardShortcuts.dom.tsx @@ -342,23 +342,3 @@ export function useKeyboardShortcuts( }; }, [eventHandlers]); } - -export function useKeyboardShortcutsConditionally( - condition: boolean, - ...eventHandlers: Array -): void { - useEffect(() => { - if (!condition) { - return; - } - - function handleKeydown(ev: KeyboardEvent): void { - eventHandlers.some(eventHandler => eventHandler(ev)); - } - - document.addEventListener('keydown', handleKeydown); - return () => { - document.removeEventListener('keydown', handleKeydown); - }; - }, [condition, eventHandlers]); -} diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json index e871cc6279..3e457b6591 100644 --- a/ts/util/lint/exceptions.json +++ b/ts/util/lint/exceptions.json @@ -2328,5 +2328,12 @@ "line": " message.innerHTML = window.SignalContext.i18n('icu:optimizingApplication');", "reasonCategory": "usageTrusted", "updated": "2021-09-17T21:02:59.414Z" + }, + { + "rule": "React-useRef", + "path": "ts/hooks/useDocumentKeyDown.dom.ts", + "line": " const listenerRef = useRef(listener);", + "reasonCategory": "usageTrusted", + "updated": "2025-12-09T15:37:49.757Z" } ]