From c77c470d2762cd7f14169270742d859d9a69d1b4 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 3 Jun 2026 12:46:14 +0530 Subject: [PATCH 1/2] test(desktop): real-DOM regression for slash/@ menu keyboard nav MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The existing slash-menu fix (PR #37937) shipped a unit test that drove the keydown reducer directly. It did not exercise the actual DOM event path — specifically the keyup-driven `refreshTrigger` that was the root cause — so it would not have caught a regression in that path. This adds a faithful @testing-library reproduction that mounts the real `useLiveCompletionAdapter` plus the index.tsx trigger wiring and fires real `keyDown` + `keyUp` event pairs on a contentEditable. It asserts: - ArrowDown cycles through ALL items (0,1,2,3,4,0,1), not just the first two - Escape closes the menu and keyup does not reopen it Reverting the fix (always-refresh keyup + unconditional setTriggerActive(0)) makes this test fail with the highlight stuck at the top — confirming it guards the real bug. --- .../composer/slash-nav-dom-repro.test.tsx | 155 ++++++++++++++++++ 1 file changed, 155 insertions(+) create mode 100644 apps/desktop/src/app/chat/composer/slash-nav-dom-repro.test.tsx diff --git a/apps/desktop/src/app/chat/composer/slash-nav-dom-repro.test.tsx b/apps/desktop/src/app/chat/composer/slash-nav-dom-repro.test.tsx new file mode 100644 index 000000000..0eac9d731 --- /dev/null +++ b/apps/desktop/src/app/chat/composer/slash-nav-dom-repro.test.tsx @@ -0,0 +1,155 @@ +import type { Unstable_TriggerAdapter, Unstable_TriggerItem } from '@assistant-ui/core' +import { act, fireEvent, render } from '@testing-library/react' +import { useCallback, useEffect, useRef, useState } from 'react' +import { describe, expect, it, vi } from 'vitest' + +import { useLiveCompletionAdapter } from './hooks/use-live-completion-adapter' +import { detectTrigger, shouldSkipTriggerRefreshOnKeyUp, type TriggerState } from './text-utils' + +// Faithful mirror of index.tsx's trigger wiring, driven through REAL DOM +// keydown+keyup events on a contentEditable. This exercises the parts the +// older direct-call repro missed: the keyup -> refreshTrigger path, the +// `refreshTrigger` reset guard, and the per-press keydown+keyup ordering. +function Harness({ + onState +}: { + onState: (s: { active: number; items: readonly Unstable_TriggerItem[]; open: boolean }) => void +}) { + const editorRef = useRef(null) + const [trigger, setTrigger] = useState(null) + const [triggerActive, setTriggerActive] = useState(0) + const [triggerItems, setTriggerItems] = useState([]) + + const { adapter } = useLiveCompletionAdapter({ + enabled: true, + debounceMs: 0, + fetcher: async (query: string) => ({ + query, + items: Array.from({ length: 5 }, (_, i) => ({ text: `/cmd${i}`, display: `/cmd${i}`, meta: '' })) + }), + toItem: (entry, index) => ({ id: `${entry.text}|${index}`, type: 'slash', label: entry.text.slice(1) }) + }) + + const triggerAdapter: Unstable_TriggerAdapter | null = trigger?.kind === '/' ? adapter : null + + const refreshTrigger = useCallback(() => { + const editor = editorRef.current + if (!editor) return + const raw = editor.textContent ?? '' + if (!raw.includes('@') && !raw.includes('/')) { + if (trigger) { + setTrigger(null) + setTriggerActive(0) + } + return + } + const detected = detectTrigger(raw) + setTrigger(detected) + if (detected?.kind !== trigger?.kind || detected?.query !== trigger?.query) { + setTriggerActive(0) + } + }, [trigger]) + + useEffect(() => { + if (!trigger || !triggerAdapter?.search) { + setTriggerItems([]) + return + } + setTriggerItems(triggerAdapter.search(trigger.query)) + }, [trigger, triggerAdapter]) + + useEffect(() => { + setTriggerActive(idx => Math.min(idx, Math.max(0, triggerItems.length - 1))) + }, [triggerItems.length]) + + onState({ active: triggerActive, items: triggerItems, open: trigger !== null }) + + // Exact copies of index.tsx handlers. + const handleKeyDown = (event: React.KeyboardEvent) => { + if (trigger && triggerItems.length > 0) { + if (event.key === 'ArrowDown') { + event.preventDefault() + setTriggerActive(idx => (idx + 1) % triggerItems.length) + return + } + if (event.key === 'ArrowUp') { + event.preventDefault() + setTriggerActive(idx => (idx - 1 + triggerItems.length) % triggerItems.length) + return + } + if (event.key === 'Escape') { + event.preventDefault() + setTrigger(null) + setTriggerItems([]) + setTriggerActive(0) + return + } + } + } + + const handleKeyUp = (event: React.KeyboardEvent) => { + if (shouldSkipTriggerRefreshOnKeyUp(event.key, trigger !== null)) return + // index.tsx uses setTimeout(refreshTrigger, 0); call synchronously here so + // the test deterministically observes the keyup-driven refresh effect. + refreshTrigger() + } + + return ( +
refreshTrigger()} + onKeyDown={handleKeyDown} + onKeyUp={handleKeyUp} + ref={editorRef} + suppressContentEditableWarning + /> + ) +} + +async function flush() { + await act(async () => { + await new Promise(r => setTimeout(r, 20)) + }) +} + +describe('slash menu navigation — real DOM keydown+keyup', () => { + it('cycles through ALL items and Esc closes, even with keyup refresh firing', async () => { + vi.useRealTimers() + let latest = { active: 0, items: [] as readonly Unstable_TriggerItem[], open: false } + const { getByTestId } = render( (latest = s)} />) + const editor = getByTestId('editor') + + // Simulate typing '/' : set text + fire input (mirrors the composer). + await act(async () => { + editor.textContent = '/' + fireEvent.input(editor) + }) + await flush() + + expect(latest.open).toBe(true) + expect(latest.items.length).toBe(5) + + // Press ArrowDown 6 times with the REAL keydown+keyup pair each time. + const seen: number[] = [latest.active] + for (let i = 0; i < 6; i++) { + await act(async () => { + fireEvent.keyDown(editor, { key: 'ArrowDown' }) + fireEvent.keyUp(editor, { key: 'ArrowDown' }) + await Promise.resolve() + }) + seen.push(latest.active) + } + + // Bug = stuck oscillating [0,1,0,1,0,1,0]. Fixed = [0,1,2,3,4,0,1]. + expect(seen).toEqual([0, 1, 2, 3, 4, 0, 1]) + + // Escape must close and stay closed (keyup must not reopen it). + await act(async () => { + fireEvent.keyDown(editor, { key: 'Escape' }) + fireEvent.keyUp(editor, { key: 'Escape' }) + await Promise.resolve() + }) + expect(latest.open).toBe(false) + }) +}) From 49f1b9e4b44f3affb33ac2d8810896d89fbbdf9f Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Wed, 3 Jun 2026 13:15:08 +0530 Subject: [PATCH 2/2] fix(desktop): stop Esc reopening the slash/@ menu; harden keyup guard Follow-up to #37937. That fix guarded the composer's keyup with `shouldSkipTriggerRefreshOnKeyUp(key, trigger !== null)`. The `trigger !== null` check is timing-fragile for Escape: Escape's *keydown* sets `trigger = null` and closes the menu, but in a real browser the *keyup* fires after a re-render, so the handler closure sees `trigger === null`, the guard returns false, `refreshTrigger` runs, re-detects the still-present `/` in the input, and instantly reopens the menu. (jsdom batches state synchronously so a unit test could not observe this -- only the running app does.) Replace the value-based guard with a `triggerKeyConsumedRef` set synchronously in keydown whenever the open popover consumes a nav/control key (Arrow/Enter/Tab/Escape). keyup consults and clears that ref, so it is immune to the keydown->re-render->keyup timing. Applied to both the main composer (chat/composer/index.tsx) and the message-edit composer (assistant-ui/thread.tsx). Removes the now-unused `shouldSkipTriggerRefreshOnKeyUp` helper and its unit test. The real-DOM regression test now fires keydown+keyup pairs through the ref-based handlers and asserts Esc closes and stays closed. Verified by running a production renderer build (Vite v8) under Electron against a local backend: ArrowDown/ArrowUp cycle the full list and Esc dismisses the menu without reopening. --- apps/desktop/src/app/chat/composer/index.tsx | 30 +++++++-- .../composer/slash-nav-dom-repro.test.tsx | 64 +++++++++++++------ .../src/app/chat/composer/text-utils.test.ts | 26 +------- .../src/app/chat/composer/text-utils.ts | 19 ------ .../src/components/assistant-ui/thread.tsx | 26 ++++++-- 5 files changed, 89 insertions(+), 76 deletions(-) diff --git a/apps/desktop/src/app/chat/composer/index.tsx b/apps/desktop/src/app/chat/composer/index.tsx index 37360f39f..aae4827ed 100644 --- a/apps/desktop/src/app/chat/composer/index.tsx +++ b/apps/desktop/src/app/chat/composer/index.tsx @@ -65,7 +65,7 @@ import { RICH_INPUT_SLOT } from './rich-editor' import { SkinSlashPopover } from './skin-slash-popover' -import { detectTrigger, extractClipboardImageBlobs, shouldSkipTriggerRefreshOnKeyUp, textBeforeCaret, type TriggerState } from './text-utils' +import { detectTrigger, extractClipboardImageBlobs, textBeforeCaret, type TriggerState } from './text-utils' import { ComposerTriggerPopover } from './trigger-popover' import type { ChatBarProps } from './types' import { UrlDialog } from './url-dialog' @@ -421,6 +421,14 @@ export function ChatBar({ const [trigger, setTrigger] = useState(null) const [triggerActive, setTriggerActive] = useState(0) const [triggerItems, setTriggerItems] = useState([]) + // Set synchronously in keydown when the open trigger popover consumes a + // navigation/control key (Arrow/Enter/Tab/Escape). The subsequent keyup must + // NOT run refreshTrigger for that keypress: it never edits text, and for + // Escape the keydown has already set trigger=null, so a keyup refresh would + // re-detect the still-present `/` and instantly reopen the menu. A ref is + // used instead of reading `trigger` in keyup because by keyup time React has + // re-rendered and the handler closure sees the post-keydown state. + const triggerKeyConsumedRef = useRef(false) const refreshTrigger = useCallback(() => { const editor = editorRef.current @@ -572,6 +580,7 @@ export function ChatBar({ if (trigger && triggerItems.length > 0) { if (event.key === 'ArrowDown') { event.preventDefault() + triggerKeyConsumedRef.current = true setTriggerActive(idx => (idx + 1) % triggerItems.length) return @@ -579,6 +588,7 @@ export function ChatBar({ if (event.key === 'ArrowUp') { event.preventDefault() + triggerKeyConsumedRef.current = true setTriggerActive(idx => (idx - 1 + triggerItems.length) % triggerItems.length) return @@ -586,6 +596,7 @@ export function ChatBar({ if (event.key === 'Enter' || event.key === 'Tab') { event.preventDefault() + triggerKeyConsumedRef.current = true const item = triggerItems[triggerActive] if (item) { @@ -597,6 +608,7 @@ export function ChatBar({ if (event.key === 'Escape') { event.preventDefault() + triggerKeyConsumedRef.current = true closeTrigger() return @@ -616,12 +628,16 @@ export function ChatBar({ } } - const handleEditorKeyUp = (event: KeyboardEvent) => { - // Arrow/Enter/Tab/Escape while the trigger menu is open are fully handled - // in keydown and never edit text. Refreshing the trigger here would reset - // the highlight to the top (breaking ArrowDown/ArrowUp) and re-open a menu - // that Escape just closed, so skip it. - if (shouldSkipTriggerRefreshOnKeyUp(event.key, trigger !== null)) { + const handleEditorKeyUp = () => { + // If this keyup belongs to a key the open trigger popover already consumed + // in keydown (Arrow/Enter/Tab/Escape), skip the refresh. Those keys never + // edit text, and for Escape the keydown already closed the menu — a refresh + // here would re-detect the still-present `/` and instantly reopen it. We + // read a ref set during keydown rather than `trigger`, because by keyup + // time React has re-rendered and `trigger` may already be null. + if (triggerKeyConsumedRef.current) { + triggerKeyConsumedRef.current = false + return } diff --git a/apps/desktop/src/app/chat/composer/slash-nav-dom-repro.test.tsx b/apps/desktop/src/app/chat/composer/slash-nav-dom-repro.test.tsx index 0eac9d731..8835d4275 100644 --- a/apps/desktop/src/app/chat/composer/slash-nav-dom-repro.test.tsx +++ b/apps/desktop/src/app/chat/composer/slash-nav-dom-repro.test.tsx @@ -4,18 +4,20 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { describe, expect, it, vi } from 'vitest' import { useLiveCompletionAdapter } from './hooks/use-live-completion-adapter' -import { detectTrigger, shouldSkipTriggerRefreshOnKeyUp, type TriggerState } from './text-utils' +import { detectTrigger, type TriggerState } from './text-utils' // Faithful mirror of index.tsx's trigger wiring, driven through REAL DOM -// keydown+keyup events on a contentEditable. This exercises the parts the -// older direct-call repro missed: the keyup -> refreshTrigger path, the -// `refreshTrigger` reset guard, and the per-press keydown+keyup ordering. +// keydown+keyup events on a contentEditable. Exercises the parts a direct +// reducer-call repro misses: the keyup -> refreshTrigger path, the +// keydown-set "consumed" ref that guards it, and per-press keydown+keyup +// ordering (critical for Escape, whose keydown nulls `trigger` before keyup). function Harness({ onState }: { onState: (s: { active: number; items: readonly Unstable_TriggerItem[]; open: boolean }) => void }) { const editorRef = useRef(null) + const triggerKeyConsumedRef = useRef(false) const [trigger, setTrigger] = useState(null) const [triggerActive, setTriggerActive] = useState(0) const [triggerItems, setTriggerItems] = useState([]) @@ -34,17 +36,22 @@ function Harness({ const refreshTrigger = useCallback(() => { const editor = editorRef.current - if (!editor) return + + if (!editor) {return} const raw = editor.textContent ?? '' + if (!raw.includes('@') && !raw.includes('/')) { if (trigger) { setTrigger(null) setTriggerActive(0) } + return } + const detected = detectTrigger(raw) setTrigger(detected) + if (detected?.kind !== trigger?.kind || detected?.query !== trigger?.query) { setTriggerActive(0) } @@ -53,8 +60,10 @@ function Harness({ useEffect(() => { if (!trigger || !triggerAdapter?.search) { setTriggerItems([]) + return } + setTriggerItems(triggerAdapter.search(trigger.query)) }, [trigger, triggerAdapter]) @@ -64,33 +73,51 @@ function Harness({ onState({ active: triggerActive, items: triggerItems, open: trigger !== null }) - // Exact copies of index.tsx handlers. + const closeTrigger = () => { + setTrigger(null) + setTriggerItems([]) + setTriggerActive(0) + } + + // Exact copies of index.tsx handlers, including the keydown-set "consumed" + // ref that the keyup consults. const handleKeyDown = (event: React.KeyboardEvent) => { if (trigger && triggerItems.length > 0) { if (event.key === 'ArrowDown') { event.preventDefault() + triggerKeyConsumedRef.current = true setTriggerActive(idx => (idx + 1) % triggerItems.length) + return } + if (event.key === 'ArrowUp') { event.preventDefault() + triggerKeyConsumedRef.current = true setTriggerActive(idx => (idx - 1 + triggerItems.length) % triggerItems.length) + return } + if (event.key === 'Escape') { event.preventDefault() - setTrigger(null) - setTriggerItems([]) - setTriggerActive(0) + triggerKeyConsumedRef.current = true + closeTrigger() + return } } } - const handleKeyUp = (event: React.KeyboardEvent) => { - if (shouldSkipTriggerRefreshOnKeyUp(event.key, trigger !== null)) return - // index.tsx uses setTimeout(refreshTrigger, 0); call synchronously here so - // the test deterministically observes the keyup-driven refresh effect. + const handleKeyUp = () => { + if (triggerKeyConsumedRef.current) { + triggerKeyConsumedRef.current = false + + return + } + + // index.tsx defers via setTimeout(refreshTrigger, 0); call synchronously + // here so the test deterministically observes the keyup-driven refresh. refreshTrigger() } @@ -114,13 +141,13 @@ async function flush() { } describe('slash menu navigation — real DOM keydown+keyup', () => { - it('cycles through ALL items and Esc closes, even with keyup refresh firing', async () => { + it('cycles through ALL items and Esc closes (and stays closed)', async () => { vi.useRealTimers() let latest = { active: 0, items: [] as readonly Unstable_TriggerItem[], open: false } const { getByTestId } = render( (latest = s)} />) const editor = getByTestId('editor') - // Simulate typing '/' : set text + fire input (mirrors the composer). + // Simulate typing '/'. await act(async () => { editor.textContent = '/' fireEvent.input(editor) @@ -130,8 +157,9 @@ describe('slash menu navigation — real DOM keydown+keyup', () => { expect(latest.open).toBe(true) expect(latest.items.length).toBe(5) - // Press ArrowDown 6 times with the REAL keydown+keyup pair each time. + // ArrowDown 6x with REAL keydown+keyup pairs. Bug = stuck [0,1,0,1,...]. const seen: number[] = [latest.active] + for (let i = 0; i < 6; i++) { await act(async () => { fireEvent.keyDown(editor, { key: 'ArrowDown' }) @@ -141,15 +169,15 @@ describe('slash menu navigation — real DOM keydown+keyup', () => { seen.push(latest.active) } - // Bug = stuck oscillating [0,1,0,1,0,1,0]. Fixed = [0,1,2,3,4,0,1]. expect(seen).toEqual([0, 1, 2, 3, 4, 0, 1]) - // Escape must close and stay closed (keyup must not reopen it). + // Escape: keydown closes; keyup must NOT reopen (the '/' is still in text). await act(async () => { fireEvent.keyDown(editor, { key: 'Escape' }) fireEvent.keyUp(editor, { key: 'Escape' }) await Promise.resolve() }) + await flush() expect(latest.open).toBe(false) }) }) diff --git a/apps/desktop/src/app/chat/composer/text-utils.test.ts b/apps/desktop/src/app/chat/composer/text-utils.test.ts index 21b433bf2..998dc8b33 100644 --- a/apps/desktop/src/app/chat/composer/text-utils.test.ts +++ b/apps/desktop/src/app/chat/composer/text-utils.test.ts @@ -1,30 +1,6 @@ import { describe, expect, it } from 'vitest' -import { detectTrigger, shouldSkipTriggerRefreshOnKeyUp } from './text-utils' - -describe('shouldSkipTriggerRefreshOnKeyUp', () => { - it('skips the trigger refresh for nav/control keys while a menu is open', () => { - // These keys are fully handled by the open-trigger keydown branch and - // never edit text. Refreshing on their keyup resets the highlight to the - // top (breaking ArrowDown/ArrowUp cycling) and re-opens a menu Escape just - // closed — the exact bugs this guard prevents. - for (const key of ['ArrowUp', 'ArrowDown', 'Enter', 'Tab', 'Escape']) { - expect(shouldSkipTriggerRefreshOnKeyUp(key, true)).toBe(true) - } - }) - - it('does not skip the refresh when no trigger menu is open', () => { - for (const key of ['ArrowUp', 'ArrowDown', 'Enter', 'Tab', 'Escape']) { - expect(shouldSkipTriggerRefreshOnKeyUp(key, false)).toBe(false) - } - }) - - it('never skips ordinary text-editing keys, so completions still refresh', () => { - for (const key of ['a', '/', '@', ' ', 'Backspace', 'ArrowLeft', 'ArrowRight']) { - expect(shouldSkipTriggerRefreshOnKeyUp(key, true)).toBe(false) - } - }) -}) +import { detectTrigger } from './text-utils' describe('detectTrigger', () => { it('detects a bare slash trigger with an empty query', () => { diff --git a/apps/desktop/src/app/chat/composer/text-utils.ts b/apps/desktop/src/app/chat/composer/text-utils.ts index db0dab977..5725883d8 100644 --- a/apps/desktop/src/app/chat/composer/text-utils.ts +++ b/apps/desktop/src/app/chat/composer/text-utils.ts @@ -8,25 +8,6 @@ export interface TriggerState { const TRIGGER_RE = /(?:^|[\s])([@/])([^\s@/]*)$/ -/** - * Keys that the open-trigger keydown handler fully consumes to drive the - * completion popover (move highlight, accept, dismiss). None of them mutate - * the editor text, so re-running `refreshTrigger` on their *keyup* is both - * useless and actively harmful: `refreshTrigger` re-detects the trigger and - * resets `triggerActive` to 0 (snapping the highlight back to the top after - * every ArrowDown/ArrowUp) and re-opens a trigger that Escape just closed. - */ -const TRIGGER_NAV_KEYS: ReadonlySet = new Set(['ArrowUp', 'ArrowDown', 'Enter', 'Tab', 'Escape']) - -/** - * True when a keyup event should NOT trigger a completion-popover refresh. - * Only applies while a trigger menu is open and the key is one the keydown - * handler already handled — guarding against the highlight-reset / reopen race. - */ -export function shouldSkipTriggerRefreshOnKeyUp(key: string, triggerOpen: boolean): boolean { - return triggerOpen && TRIGGER_NAV_KEYS.has(key) -} - export function extractClipboardImageBlobs(clipboard: DataTransfer): Blob[] { const blobs: Blob[] = [] const seen = new Set() diff --git a/apps/desktop/src/components/assistant-ui/thread.tsx b/apps/desktop/src/components/assistant-ui/thread.tsx index 0eede6204..1576ab9b5 100644 --- a/apps/desktop/src/components/assistant-ui/thread.tsx +++ b/apps/desktop/src/components/assistant-ui/thread.tsx @@ -44,7 +44,7 @@ import { renderComposerContents, RICH_INPUT_SLOT } from '@/app/chat/composer/rich-editor' -import { detectTrigger, shouldSkipTriggerRefreshOnKeyUp, textBeforeCaret, type TriggerState } from '@/app/chat/composer/text-utils' +import { detectTrigger, textBeforeCaret, type TriggerState } from '@/app/chat/composer/text-utils' import { ComposerTriggerPopover } from '@/app/chat/composer/trigger-popover' import { extractDroppedFiles, HERMES_PATHS_MIME } from '@/app/chat/hooks/use-composer-actions' import { ClarifyTool } from '@/components/assistant-ui/clarify-tool' @@ -873,6 +873,10 @@ const UserEditComposer: FC = ({ cwd, gateway, sessionId } const [trigger, setTrigger] = useState(null) const [triggerActive, setTriggerActive] = useState(0) const [triggerItems, setTriggerItems] = useState([]) + // See index.tsx: set in keydown when the open popover consumes a nav/control + // key so the matching keyup skips refreshTrigger (timing-immune vs reading + // `trigger`, which keyup sees as already-null after Escape). + const triggerKeyConsumedRef = useRef(false) const [triggerPlacement, setTriggerPlacement] = useState<'bottom' | 'top'>('top') const [focusRequestId, setFocusRequestId] = useState(0) const [submitting, setSubmitting] = useState(false) @@ -1238,6 +1242,7 @@ const UserEditComposer: FC = ({ cwd, gateway, sessionId } if (trigger && triggerItems.length > 0) { if (event.key === 'ArrowDown') { event.preventDefault() + triggerKeyConsumedRef.current = true setTriggerActive(idx => (idx + 1) % triggerItems.length) return @@ -1245,6 +1250,7 @@ const UserEditComposer: FC = ({ cwd, gateway, sessionId } if (event.key === 'ArrowUp') { event.preventDefault() + triggerKeyConsumedRef.current = true setTriggerActive(idx => (idx - 1 + triggerItems.length) % triggerItems.length) return @@ -1252,6 +1258,7 @@ const UserEditComposer: FC = ({ cwd, gateway, sessionId } if (event.key === 'Enter' || event.key === 'Tab') { event.preventDefault() + triggerKeyConsumedRef.current = true const item = triggerItems[triggerActive] if (item) { @@ -1263,6 +1270,7 @@ const UserEditComposer: FC = ({ cwd, gateway, sessionId } if (event.key === 'Escape') { event.preventDefault() + triggerKeyConsumedRef.current = true closeTrigger() return @@ -1282,12 +1290,16 @@ const UserEditComposer: FC = ({ cwd, gateway, sessionId } } } - const handleKeyUp = (event: KeyboardEvent) => { - // Arrow/Enter/Tab/Escape while the trigger menu is open are fully handled - // in keydown and never edit text. Refreshing the trigger here would reset - // the highlight to the top (breaking ArrowDown/ArrowUp) and re-open a menu - // that Escape just closed, so skip it. - if (shouldSkipTriggerRefreshOnKeyUp(event.key, trigger !== null)) { + const handleKeyUp = () => { + // If this keyup belongs to a key the open trigger popover already consumed + // in keydown (Arrow/Enter/Tab/Escape), skip the refresh. Those keys never + // edit text, and for Escape the keydown already closed the menu — a refresh + // here would re-detect the still-present `/` and instantly reopen it. We + // read a ref set during keydown rather than `trigger`, because by keyup + // time React has re-rendered and `trigger` may already be null. + if (triggerKeyConsumedRef.current) { + triggerKeyConsumedRef.current = false + return }