mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 09:32:24 +00:00
🫴 fix: Hand Composer Focus Back After Upload Dialogs (#15469)
Typing a message, pasting an image, then pressing Enter did nothing: the composer held the full draft, the send button was enabled, and Enter was inert until the user clicked back into the textarea — which reads as "the textarea needs a text change before it will submit". The send gating was never at fault. Radix's modal `DialogContent` always cancels its default close-focus restore and focuses `context.triggerRef.current` instead. `DragDropModal` — the "Select Upload Type" dialog a paste or a drop opens — is opened programmatically and so has no `Dialog.Trigger`, leaving that ref null: nothing is focused and focus falls to `document.body` when the dialog unmounts. Enter-to-send is a keydown handler on the textarea, so from `body` it is never reached. `UploadModalProvider` now records the element the upload was started from, captured in `openModal` before the dialog mounts (an effect runs too late — Radix has already moved focus), and restores it once the dialog closes, queued behind Radix's own deferred restore. This covers every close path: picking a destination, Cancel, Escape, and dismissing the overlay. `OGDialog`'s existing `triggerRef` seam does not reach this case, since it only fires through Radix's `onOpenChange` and choosing an upload destination calls `closeModal()` directly.
This commit is contained in:
parent
c3beae1e18
commit
537ddba6bb
2 changed files with 249 additions and 1 deletions
|
|
@ -1,4 +1,12 @@
|
|||
import React, { createContext, useCallback, useContext, useMemo, useState } from 'react';
|
||||
import React, {
|
||||
useRef,
|
||||
useMemo,
|
||||
useState,
|
||||
useEffect,
|
||||
useContext,
|
||||
useCallback,
|
||||
createContext,
|
||||
} from 'react';
|
||||
|
||||
interface UploadModalContextValue {
|
||||
isVisible: boolean;
|
||||
|
|
@ -19,8 +27,20 @@ const UploadModalContext = createContext<UploadModalContextValue>(defaultValue);
|
|||
export function UploadModalProvider({ children }: { children: React.ReactNode }) {
|
||||
const [isVisible, setIsVisible] = useState(false);
|
||||
const [files, setFiles] = useState<File[]>([]);
|
||||
/**
|
||||
* A paste or a drop opens this dialog programmatically, so there is no
|
||||
* `Dialog.Trigger` for Radix to hand focus back to — and its modal content
|
||||
* always cancels the default restore in favour of that missing trigger,
|
||||
* dropping focus on `document.body`. The composer's Enter-to-send is a
|
||||
* textarea key handler, so the user is left holding a full draft and an
|
||||
* enabled send button that Enter no longer reaches, until they click back
|
||||
* into the composer. Remember where the upload was started from instead.
|
||||
*/
|
||||
const returnFocusRef = useRef<HTMLElement | null>(null);
|
||||
|
||||
const openModal = useCallback((nextFiles: File[]) => {
|
||||
const active = document.activeElement;
|
||||
returnFocusRef.current = active instanceof HTMLElement ? active : null;
|
||||
setFiles(nextFiles);
|
||||
setIsVisible(true);
|
||||
}, []);
|
||||
|
|
@ -30,6 +50,22 @@ export function UploadModalProvider({ children }: { children: React.ReactNode })
|
|||
setFiles([]);
|
||||
}, []);
|
||||
|
||||
useEffect(() => {
|
||||
if (isVisible) {
|
||||
return;
|
||||
}
|
||||
const element = returnFocusRef.current;
|
||||
returnFocusRef.current = null;
|
||||
if (element == null || !element.isConnected) {
|
||||
return;
|
||||
}
|
||||
/** Radix restores focus from the dialog's own unmount cleanup, deferred by
|
||||
* a timeout of its own. This effect runs after that cleanup, so the queued
|
||||
* restore lands last. */
|
||||
const timeout = setTimeout(() => element.focus({ preventScroll: true }), 0);
|
||||
return () => clearTimeout(timeout);
|
||||
}, [isVisible]);
|
||||
|
||||
const value = useMemo<UploadModalContextValue>(
|
||||
() => ({ isVisible, files, openModal, closeModal }),
|
||||
[isVisible, files, openModal, closeModal],
|
||||
|
|
|
|||
|
|
@ -0,0 +1,212 @@
|
|||
import React, { useMemo, useState } from 'react';
|
||||
import '@testing-library/jest-dom';
|
||||
import { DndProvider } from 'react-dnd';
|
||||
import { useForm } from 'react-hook-form';
|
||||
import { RecoilRoot, useRecoilState } from 'recoil';
|
||||
import userEvent from '@testing-library/user-event';
|
||||
import { HTML5Backend } from 'react-dnd-html5-backend';
|
||||
import { BrowserRouter as Router } from 'react-router-dom';
|
||||
import { QueryClient, QueryClientProvider } from '@tanstack/react-query';
|
||||
import { render, screen, waitFor, fireEvent } from '@testing-library/react';
|
||||
import { QueryKeys, FileSources, EModelEndpoint } from 'librechat-data-provider';
|
||||
import type { TFile, TFileUpload, TConversation } from 'librechat-data-provider';
|
||||
import type { ChatFormValues } from '~/common';
|
||||
import { ChatContext, ChatFormProvider } from '~/Providers';
|
||||
import { AuthContextProvider } from '~/hooks/AuthContext';
|
||||
import DragDropWrapper from '../Files/DragDropWrapper';
|
||||
import ChatForm from '../ChatForm';
|
||||
import store from '~/store';
|
||||
|
||||
const mockUpload = jest.fn();
|
||||
const mockAsk = jest.fn();
|
||||
|
||||
jest.mock('librechat-data-provider', () => {
|
||||
const actual = jest.requireActual('librechat-data-provider');
|
||||
return {
|
||||
...actual,
|
||||
dataService: {
|
||||
...actual.dataService,
|
||||
uploadImage: (...args: unknown[]) => mockUpload(...args),
|
||||
uploadFile: (...args: unknown[]) => mockUpload(...args),
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
const conversation = {
|
||||
conversationId: 'new',
|
||||
endpoint: EModelEndpoint.openAI,
|
||||
model: 'gpt-4o',
|
||||
title: 'New Chat',
|
||||
} as TConversation;
|
||||
|
||||
const uploadResponse = {
|
||||
message: 'File uploaded',
|
||||
file_id: 'server-file-id',
|
||||
temp_file_id: 'temp-file-id',
|
||||
filename: 'pasted.png',
|
||||
filepath: '/images/pasted.png',
|
||||
type: 'image/png',
|
||||
bytes: 2048,
|
||||
height: 100,
|
||||
width: 100,
|
||||
source: FileSources.local,
|
||||
embedded: false,
|
||||
} as unknown as TFileUpload;
|
||||
|
||||
/** jsdom never decodes images; the app only starts the upload once one loads. */
|
||||
class StubImage {
|
||||
onload: (() => void) | null = null;
|
||||
onerror: (() => void) | null = null;
|
||||
width = 100;
|
||||
height = 100;
|
||||
set src(_value: string) {
|
||||
setTimeout(() => this.onload?.(), 0);
|
||||
}
|
||||
}
|
||||
|
||||
function Harness() {
|
||||
const [files, setFiles] = useRecoilState(store.filesByIndex(0));
|
||||
const [isSubmitting] = useRecoilState(store.isSubmittingFamily(0));
|
||||
const [, setFilesLoading] = useState(false);
|
||||
const methods = useForm<ChatFormValues>({ defaultValues: { text: '' } });
|
||||
|
||||
const chatHelpers = useMemo(
|
||||
() =>
|
||||
({
|
||||
index: 0,
|
||||
conversation,
|
||||
setConversation: () => undefined,
|
||||
files,
|
||||
setFiles,
|
||||
isSubmitting,
|
||||
setIsSubmitting: () => undefined,
|
||||
filesLoading: false,
|
||||
setFilesLoading,
|
||||
newConversation: () => undefined,
|
||||
handleStopGenerating: () => undefined,
|
||||
stopGenerating: () => undefined,
|
||||
getMessages: () => [],
|
||||
setMessages: () => undefined,
|
||||
ask: mockAsk,
|
||||
regenerate: () => undefined,
|
||||
setSiblingIdx: () => undefined,
|
||||
showPopover: false,
|
||||
setShowPopover: () => undefined,
|
||||
abortScroll: false,
|
||||
setAbortScroll: () => undefined,
|
||||
preset: null,
|
||||
setPreset: () => undefined,
|
||||
optionSettings: {},
|
||||
setOptionSettings: () => undefined,
|
||||
handleRegenerate: () => undefined,
|
||||
handleContinue: () => undefined,
|
||||
}) as unknown as React.ContextType<typeof ChatContext>,
|
||||
[files, setFiles, isSubmitting],
|
||||
);
|
||||
|
||||
return (
|
||||
<ChatFormProvider {...methods}>
|
||||
<ChatContext.Provider value={chatHelpers}>
|
||||
<DragDropWrapper>
|
||||
<ChatForm index={0} />
|
||||
</DragDropWrapper>
|
||||
</ChatContext.Provider>
|
||||
</ChatFormProvider>
|
||||
);
|
||||
}
|
||||
|
||||
function renderComposer() {
|
||||
const queryClient = new QueryClient({
|
||||
defaultOptions: { queries: { retry: false }, mutations: { retry: false } },
|
||||
});
|
||||
queryClient.setQueryData([QueryKeys.fileConfig], {});
|
||||
queryClient.setQueryData<TFile[]>([QueryKeys.files], []);
|
||||
queryClient.setQueryData([QueryKeys.endpoints], { [EModelEndpoint.openAI]: { order: 0 } });
|
||||
|
||||
return render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<RecoilRoot>
|
||||
<Router>
|
||||
<AuthContextProvider authConfig={{ loginRedirect: '', test: true }}>
|
||||
<DndProvider backend={HTML5Backend}>
|
||||
<Harness />
|
||||
</DndProvider>
|
||||
</AuthContextProvider>
|
||||
</Router>
|
||||
</RecoilRoot>
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
}
|
||||
|
||||
const pasteImage = (textarea: HTMLElement) => {
|
||||
const file = new File(['image-bytes'], 'pasted.png', { type: 'image/png' });
|
||||
const event = new Event('paste', { bubbles: true, cancelable: true });
|
||||
Object.defineProperty(event, 'clipboardData', {
|
||||
value: {
|
||||
files: [file],
|
||||
items: [{ kind: 'file', type: 'image/png', getAsFile: () => file }],
|
||||
types: ['Files'],
|
||||
getData: () => '',
|
||||
},
|
||||
});
|
||||
fireEvent(textarea, event);
|
||||
};
|
||||
|
||||
/**
|
||||
* The upload-destination dialog is opened by the paste itself, so Radix has no
|
||||
* trigger to return focus to when it closes and used to leave focus on
|
||||
* `document.body` — which reads to the user as "the send button is enabled but
|
||||
* Enter does nothing", since Enter-to-send is a textarea key handler.
|
||||
*/
|
||||
describe('composer focus after a pasted upload', () => {
|
||||
beforeEach(() => {
|
||||
localStorage.clear();
|
||||
global.URL.createObjectURL = jest.fn(() => 'blob:preview');
|
||||
global.URL.revokeObjectURL = jest.fn();
|
||||
(global as unknown as { Image: unknown }).Image = StubImage;
|
||||
mockUpload.mockReset();
|
||||
mockAsk.mockReset();
|
||||
mockUpload.mockImplementation((body: FormData) =>
|
||||
Promise.resolve({ ...uploadResponse, temp_file_id: body.get('file_id') as string }),
|
||||
);
|
||||
});
|
||||
|
||||
test('returns focus to the composer when the upload dialog closes', async () => {
|
||||
renderComposer();
|
||||
|
||||
const textarea = await screen.findByTestId('text-input');
|
||||
await userEvent.click(textarea);
|
||||
await userEvent.type(textarea, 'hi');
|
||||
expect(textarea).toHaveFocus();
|
||||
|
||||
pasteImage(textarea);
|
||||
const [option] = await screen.findAllByRole('button', { name: /upload/i });
|
||||
expect(textarea).not.toHaveFocus();
|
||||
|
||||
await userEvent.click(option);
|
||||
await waitFor(() => expect(mockUpload).toHaveBeenCalled());
|
||||
await waitFor(() => expect(textarea).toHaveFocus());
|
||||
}, 20000);
|
||||
|
||||
test('Enter still sends after a pasted upload, with no further typing', async () => {
|
||||
renderComposer();
|
||||
|
||||
const textarea = await screen.findByTestId('text-input');
|
||||
await userEvent.click(textarea);
|
||||
await userEvent.type(textarea, 'hi');
|
||||
|
||||
pasteImage(textarea);
|
||||
const [option] = await screen.findAllByRole('button', { name: /upload/i });
|
||||
await userEvent.click(option);
|
||||
await waitFor(() => expect(mockUpload).toHaveBeenCalled());
|
||||
await waitFor(() => expect(screen.getByTestId('send-button')).toBeEnabled());
|
||||
await waitFor(() => expect(textarea).toHaveFocus());
|
||||
|
||||
/** Straight to the keyboard: clicking or typing first would restore focus
|
||||
* on its own and hide the regression this covers. */
|
||||
fireEvent.keyDown(document.activeElement as Element, { key: 'Enter', code: 'Enter' });
|
||||
|
||||
await waitFor(() => expect(mockAsk).toHaveBeenCalled());
|
||||
expect(mockAsk.mock.calls[0][0]).toEqual(expect.objectContaining({ text: 'hi' }));
|
||||
}, 20000);
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue