From ef1ee6ee1667777e0a93f6ca1696b3cb66c2b9d7 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 23 Jun 2026 23:14:24 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=A4=20fix:=20Guard=20Prompts=20Popover?= =?UTF-8?q?=20Against=20Empty=20Result=20Keyboard=20Navigation=20(#13931)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🛡️ fix: Guard Prompts and Mention popovers against empty-result navigation * 🛡️ fix: Prevent Tab default and clear stale filter on empty popover close --- client/src/components/Chat/Input/Mention.tsx | 6 +- .../components/Chat/Input/PromptsCommand.tsx | 20 +- .../Chat/Input/__tests__/Mention.spec.tsx | 208 +++++++++++++++++ .../Input/__tests__/PromptsCommand.spec.tsx | 220 ++++++++++++++++++ 4 files changed, 449 insertions(+), 5 deletions(-) create mode 100644 client/src/components/Chat/Input/__tests__/Mention.spec.tsx create mode 100644 client/src/components/Chat/Input/__tests__/PromptsCommand.spec.tsx diff --git a/client/src/components/Chat/Input/Mention.tsx b/client/src/components/Chat/Input/Mention.tsx index 464a0afc87..a188503478 100644 --- a/client/src/components/Chat/Input/Mention.tsx +++ b/client/src/components/Chat/Input/Mention.tsx @@ -1,8 +1,8 @@ import { memo, useState, useRef, useEffect } from 'react'; -import { useRecoilValue, useSetRecoilState } from 'recoil'; import { AutoSizer, List } from 'react-virtualized'; import { Spinner, useCombobox } from '@librechat/client'; import { EModelEndpoint } from 'librechat-data-provider'; +import { useRecoilValue, useSetRecoilState } from 'recoil'; import type { RecoilState } from 'recoil'; import type { MentionOption, ConvoGenerator } from '~/common'; import { useGetConversation, useLocalize, TranslationKeys } from '~/hooks'; @@ -206,9 +206,7 @@ function MentionContent({ setActiveIndex((prevIndex) => (prevIndex - 1 + matches.length) % matches.length); } else if (e.key === 'Enter' || e.key === 'Tab') { if (matches.length === 0) { - if (e.key === 'Enter') { - e.preventDefault(); - } + e.preventDefault(); setOpen(false); setShowPopover(false); textAreaRef.current?.focus(); diff --git a/client/src/components/Chat/Input/PromptsCommand.tsx b/client/src/components/Chat/Input/PromptsCommand.tsx index 6db9eb62bc..4efa7a1a7f 100644 --- a/client/src/components/Chat/Input/PromptsCommand.tsx +++ b/client/src/components/Chat/Input/PromptsCommand.tsx @@ -138,10 +138,15 @@ function PromptsCommand({ useEffect(() => { if (!open) { setActiveIndex(0); + setSearchValue(''); } else { setVariableGroup(null); } - }, [open]); + }, [open, setSearchValue]); + + useEffect(() => { + setActiveIndex((prev) => Math.min(prev, Math.max(matches.length - 1, 0))); + }, [matches.length]); useEffect(() => { return () => { @@ -214,10 +219,23 @@ function PromptsCommand({ textAreaRef.current?.focus(); } if (e.key === 'ArrowDown') { + if (matches.length === 0) { + return; + } setActiveIndex((prevIndex) => (prevIndex + 1) % matches.length); } else if (e.key === 'ArrowUp') { + if (matches.length === 0) { + return; + } setActiveIndex((prevIndex) => (prevIndex - 1 + matches.length) % matches.length); } else if (e.key === 'Enter' || e.key === 'Tab') { + if (matches.length === 0) { + e.preventDefault(); + setOpen(false); + setShowPromptsPopover(false); + textAreaRef.current?.focus(); + return; + } if (e.key === 'Enter') { e.preventDefault(); } diff --git a/client/src/components/Chat/Input/__tests__/Mention.spec.tsx b/client/src/components/Chat/Input/__tests__/Mention.spec.tsx new file mode 100644 index 0000000000..d5a9d863a0 --- /dev/null +++ b/client/src/components/Chat/Input/__tests__/Mention.spec.tsx @@ -0,0 +1,208 @@ +/** + * Locks in the `@` mention popover keyboard contract for the empty-result + * edge case (issue #13929 / PR #13928): when filtering yields zero matches + * the arrow keys must be no-ops rather than computing a `% 0` -> NaN active + * index, and Enter/Tab must close the popover and return focus to the + * textarea. Deleting back to a non-empty list has to leave keyboard + * navigation working from a valid index. + */ +import React from 'react'; +import { fireEvent, render, screen } from '@testing-library/react'; +import type { RecoilState } from 'recoil'; +import type { MentionOption } from '~/common'; + +const POPOVER_ATOM = 'show-mention-popover'; +const PLACEHOLDER = 'com_ui_mention'; + +const mockSetShowPopover = jest.fn(); +const mockShowPopover = { current: true }; + +jest.mock('recoil', () => { + const actual = jest.requireActual('recoil'); + return { + ...actual, + useRecoilValue: jest.fn((atom: unknown) => + atom === POPOVER_ATOM ? mockShowPopover.current : undefined, + ), + useSetRecoilState: jest.fn((atom: unknown) => + atom === POPOVER_ATOM ? mockSetShowPopover : jest.fn(), + ), + }; +}); + +const mockUseMentions = jest.fn(); +jest.mock('~/hooks/Input/useMentions', () => ({ + __esModule: true, + default: () => mockUseMentions(), +})); + +jest.mock('~/hooks/Input/useSelectMention', () => ({ + __esModule: true, + default: () => ({ onSelectMention: jest.fn() }), +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, + useGetConversation: () => jest.fn(), +})); + +jest.mock('~/Providers', () => ({ + useAssistantsMapContext: () => ({}), +})); + +/* react-virtualized renders nothing in jsdom without a measured size; replace + AutoSizer + List with a flat ul so every row's MentionItem button renders. */ +jest.mock('react-virtualized', () => ({ + ...jest.requireActual('react-virtualized'), + AutoSizer: ({ children }: { children: (size: { width: number }) => React.ReactNode }) => + children({ width: 320 }), + List: ({ + rowCount, + rowRenderer, + }: { + rowCount: number; + rowRenderer: (args: { + index: number; + key: string; + style: React.CSSProperties; + }) => React.ReactNode; + }) => { + const rows: React.ReactNode[] = []; + for (let i = 0; i < rowCount; i++) { + rows.push(rowRenderer({ index: i, key: `row-${i}`, style: {} })); + } + return ; + }, +})); + +import Mention from '../Mention'; + +const makeTextarea = (initial = '@') => { + const textarea = document.createElement('textarea'); + textarea.value = initial; + document.body.appendChild(textarea); + return { current: textarea } as React.MutableRefObject; +}; + +const makeMention = (overrides: Partial): MentionOption => ({ + type: 'preset', + label: 'Alpha', + value: 'alpha', + description: '', + ...overrides, +}); + +const options: MentionOption[] = [ + makeMention({ label: 'Alpha', value: 'alpha' }), + makeMention({ label: 'Beta', value: 'beta' }), +]; + +const mentionsBundle = { + options, + presets: [], + isLoading: false, + modelSpecs: [], + agentsList: [], + modelsConfig: {}, + endpointsConfig: {}, + assistantListMap: {}, +}; + +const activeItemId = () => + document.querySelector('.bg-surface-active')?.closest('button')?.id ?? null; + +const getInput = () => screen.getByPlaceholderText(PLACEHOLDER); + +beforeAll(() => { + Element.prototype.scrollIntoView = jest.fn(); +}); + +beforeEach(() => { + jest.clearAllMocks(); + document.body.innerHTML = ''; + mockShowPopover.current = true; + mockUseMentions.mockReturnValue(mentionsBundle); +}); + +const renderMention = () => { + const textAreaRef = makeTextarea('@'); + const utils = render( + } + newConversation={jest.fn()} + textAreaRef={textAreaRef} + />, + ); + return { textAreaRef, ...utils }; +}; + +describe('Mention keyboard navigation', () => { + it('renders nothing when the popover atom is false', () => { + mockShowPopover.current = false; + const { container } = renderMention(); + expect(container).toBeEmptyDOMElement(); + }); + + it('moves the active highlight with ArrowDown/ArrowUp and wraps around', () => { + renderMention(); + const input = getInput(); + + expect(activeItemId()).toBe('mention-item-0'); + + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(activeItemId()).toBe('mention-item-1'); + + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(activeItemId()).toBe('mention-item-0'); + + fireEvent.keyDown(input, { key: 'ArrowUp' }); + expect(activeItemId()).toBe('mention-item-1'); + }); + + it('keeps navigation working after filtering to zero matches and back', () => { + renderMention(); + const input = getInput(); + + fireEvent.change(input, { target: { value: 'zzz' } }); + expect(screen.queryByRole('button')).toBeNull(); + + /* Arrow keys on an empty list must be no-ops, not `% 0` -> NaN. */ + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.keyDown(input, { key: 'ArrowUp' }); + + fireEvent.change(input, { target: { value: '' } }); + expect(screen.getAllByRole('button')).toHaveLength(2); + + /* Without the guard the active index would still be NaN here and no + item would highlight; with it, navigation resumes from a valid index. */ + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(activeItemId()).toBe('mention-item-1'); + }); + + it('closes the popover and refocuses the textarea when Enter is pressed with no matches', () => { + const { textAreaRef } = renderMention(); + const input = getInput(); + + fireEvent.change(input, { target: { value: 'zzz' } }); + const notPrevented = fireEvent.keyDown(input, { key: 'Enter' }); + + expect(notPrevented).toBe(false); + expect(mockSetShowPopover).toHaveBeenCalledWith(false); + expect(document.activeElement).toBe(textAreaRef.current); + }); + + it('prevents the default Tab action when closing on no matches so the refocus sticks', () => { + const { textAreaRef } = renderMention(); + const input = getInput(); + + fireEvent.change(input, { target: { value: 'zzz' } }); + const notPrevented = fireEvent.keyDown(input, { key: 'Tab' }); + + /* Without preventDefault the browser's default Tab would move focus off + the textarea right after we refocus it. */ + expect(notPrevented).toBe(false); + expect(mockSetShowPopover).toHaveBeenCalledWith(false); + expect(document.activeElement).toBe(textAreaRef.current); + }); +}); diff --git a/client/src/components/Chat/Input/__tests__/PromptsCommand.spec.tsx b/client/src/components/Chat/Input/__tests__/PromptsCommand.spec.tsx new file mode 100644 index 0000000000..adf9f63716 --- /dev/null +++ b/client/src/components/Chat/Input/__tests__/PromptsCommand.spec.tsx @@ -0,0 +1,220 @@ +/** + * Locks in the `/` prompts popover keyboard contract, specifically the + * empty-result edge case that mirrors the `$` skills command and the `@` + * mention popover: when filtering yields zero matches the arrow keys must + * be no-ops (never `% 0` into a `NaN` active index) and Enter/Tab must + * close the popover and return focus to the textarea. Deleting back to a + * non-empty list has to leave keyboard navigation working. + */ +import React from 'react'; +import { fireEvent, render, screen } from '@testing-library/react'; +import type { PromptOption } from '~/common'; + +const PLACEHOLDER = 'com_ui_command_usage_placeholder'; + +const mockSetShowPromptsPopover = jest.fn(); +const mockShowPromptsPopover = { current: true }; + +jest.mock('recoil', () => { + const actual = jest.requireActual('recoil'); + return { + ...actual, + useRecoilValue: jest.fn((atom: unknown) => + atom === 'show-prompts-popover' ? mockShowPromptsPopover.current : undefined, + ), + useSetRecoilState: jest.fn((atom: unknown) => + atom === 'show-prompts-popover' ? mockSetShowPromptsPopover : jest.fn(), + ), + }; +}); + +jest.mock('~/store', () => ({ + __esModule: true, + default: { + showPromptsPopoverFamily: () => 'show-prompts-popover', + }, +})); + +const mockRecordUsage = jest.fn(); +jest.mock('~/data-provider', () => ({ + useRecordPromptUsage: () => ({ mutate: mockRecordUsage }), +})); + +const mockPromptGroupsContext = jest.fn(); +jest.mock('~/Providers', () => ({ + usePromptGroupsContext: () => mockPromptGroupsContext(), +})); + +jest.mock('~/components/Prompts', () => ({ + VariableDialog: () => null, +})); + +jest.mock('~/hooks', () => ({ + useLocalize: () => (key: string) => key, +})); + +/* react-virtualized renders nothing in jsdom without a measured size; replace + AutoSizer + List with a flat ul so every row's MentionItem button renders. */ +jest.mock('react-virtualized', () => ({ + ...jest.requireActual('react-virtualized'), + AutoSizer: ({ children }: { children: (size: { width: number }) => React.ReactNode }) => + children({ width: 320 }), + List: ({ + rowCount, + rowRenderer, + }: { + rowCount: number; + rowRenderer: (args: { + index: number; + key: string; + style: React.CSSProperties; + }) => React.ReactNode; + }) => { + const rows: React.ReactNode[] = []; + for (let i = 0; i < rowCount; i++) { + rows.push(rowRenderer({ index: i, key: `row-${i}`, style: {} })); + } + return
    {rows}
; + }, +})); + +import PromptsCommand from '../PromptsCommand'; + +const makeTextarea = (initial = '/') => { + const textarea = document.createElement('textarea'); + textarea.value = initial; + document.body.appendChild(textarea); + return { current: textarea } as React.MutableRefObject; +}; + +const makePrompt = (overrides: Partial): PromptOption => ({ + id: '1', + type: 'prompt', + label: 'Alpha', + value: 'Alpha', + description: '', + ...overrides, +}); + +const promptGroups: PromptOption[] = [ + makePrompt({ id: '1', label: 'Alpha', value: 'Alpha' }), + makePrompt({ id: '2', label: 'Beta', value: 'Beta' }), +]; + +const promptsMap = { + '1': { _id: '1', productionPrompt: { prompt: 'Alpha body' } }, + '2': { _id: '2', productionPrompt: { prompt: 'Beta body' } }, +}; + +const activeItemId = () => + document.querySelector('.bg-surface-active')?.closest('button')?.id ?? null; + +const getInput = () => screen.getByPlaceholderText(PLACEHOLDER); + +beforeAll(() => { + Element.prototype.scrollIntoView = jest.fn(); +}); + +beforeEach(() => { + jest.clearAllMocks(); + document.body.innerHTML = ''; + mockShowPromptsPopover.current = true; + mockPromptGroupsContext.mockReturnValue({ + hasAccess: true, + allPromptGroups: { + data: { promptGroups, promptsMap }, + isLoading: false, + }, + }); +}); + +const renderCommand = () => { + const textAreaRef = makeTextarea('/'); + const utils = render( + , + ); + return { textAreaRef, ...utils }; +}; + +describe('PromptsCommand keyboard navigation', () => { + it('renders nothing when the popover atom is false', () => { + mockShowPromptsPopover.current = false; + const { container } = renderCommand(); + expect(container).toBeEmptyDOMElement(); + }); + + it('moves the active highlight with ArrowDown/ArrowUp and wraps around', () => { + renderCommand(); + const input = getInput(); + + expect(activeItemId()).toBe('prompt-item-0'); + + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(activeItemId()).toBe('prompt-item-1'); + + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(activeItemId()).toBe('prompt-item-0'); + + fireEvent.keyDown(input, { key: 'ArrowUp' }); + expect(activeItemId()).toBe('prompt-item-1'); + }); + + it('keeps navigation working after filtering to zero matches and back', () => { + renderCommand(); + const input = getInput(); + + fireEvent.change(input, { target: { value: 'zzz' } }); + expect(screen.queryByRole('button')).toBeNull(); + + /* Arrow keys on an empty list must be no-ops, not `% 0` -> NaN. */ + fireEvent.keyDown(input, { key: 'ArrowDown' }); + fireEvent.keyDown(input, { key: 'ArrowUp' }); + + fireEvent.change(input, { target: { value: '' } }); + expect(screen.getAllByRole('button')).toHaveLength(2); + + /* Without the guard the active index would still be NaN here and no + item would highlight; with it, navigation resumes from a valid index. */ + fireEvent.keyDown(input, { key: 'ArrowDown' }); + expect(activeItemId()).toBe('prompt-item-1'); + }); + + it('closes the popover and refocuses the textarea when Enter is pressed with no matches', () => { + const { textAreaRef } = renderCommand(); + const input = getInput(); + + fireEvent.change(input, { target: { value: 'zzz' } }); + const notPrevented = fireEvent.keyDown(input, { key: 'Enter' }); + + expect(notPrevented).toBe(false); + expect(mockSetShowPromptsPopover).toHaveBeenCalledWith(false); + expect(document.activeElement).toBe(textAreaRef.current); + }); + + it('prevents the default Tab action when closing on no matches so the refocus sticks', () => { + const { textAreaRef } = renderCommand(); + const input = getInput(); + + fireEvent.change(input, { target: { value: 'zzz' } }); + const notPrevented = fireEvent.keyDown(input, { key: 'Tab' }); + + /* Without preventDefault the browser's default Tab would move focus off + the textarea right after we refocus it. */ + expect(notPrevented).toBe(false); + expect(mockSetShowPromptsPopover).toHaveBeenCalledWith(false); + expect(document.activeElement).toBe(textAreaRef.current); + }); + + it('clears the stale search filter when the popover closes', () => { + renderCommand(); + const input = getInput(); + + fireEvent.change(input, { target: { value: 'zzz' } }); + expect((input as HTMLInputElement).value).toBe('zzz'); + + /* PromptsCommand stays mounted across close (unlike Mention), so a leftover + no-match query has to be cleared or the popover reopens still filtered. */ + fireEvent.keyDown(input, { key: 'Enter' }); + expect((getInput() as HTMLInputElement).value).toBe(''); + }); +});