mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-21 17:44:11 +00:00
🎨 fix: Match In-Flight Steer Presentation to the Applied Part
Codex review on 6a5f36f7ef. All three findings were real, and all three were the same underlying mistake: the anchored bubble hand-rolled presentation instead of reusing the leaves the applied SteerPart uses, so a steer visibly changed on apply. - Images: the message `Image` sets an inline height from the file's dimensions and centers with object-contain, so clipping it into a 56px wrapper showed the blank top of a large element. Use ImagePreview, the composer's fixed-size thumbnail path (also gives click-to-enlarge). - Non-image files: FileContainer always renders a button, so without an onClick the chip was dead. Wire FilePreviewDialog, as SteerPart does. - Markdown: honor enableUserMsgMarkdown so text does not reflow the moment the server injects it. Splits files in a single pass rather than two filters.
This commit is contained in:
parent
a53b7f62f8
commit
8135c808df
2 changed files with 146 additions and 32 deletions
|
|
@ -1,14 +1,25 @@
|
|||
import { memo } from 'react';
|
||||
import { memo, useMemo, useState, useCallback } from 'react';
|
||||
import { X, Zap } from 'lucide-react';
|
||||
import { useRecoilValue } from 'recoil';
|
||||
import type { TFile } from 'librechat-data-provider';
|
||||
import type { TFile, TMessage } from 'librechat-data-provider';
|
||||
import type { PendingSteer } from '~/store/families';
|
||||
import FilePreviewDialog from '~/components/Chat/Messages/Content/FilePreviewDialog';
|
||||
import MarkdownLite from '~/components/Chat/Messages/Content/MarkdownLite';
|
||||
import FileContainer from '~/components/Chat/Input/Files/FileContainer';
|
||||
import Image from '~/components/Chat/Messages/Content/Image';
|
||||
import ImagePreview from '~/components/Chat/Input/Files/ImagePreview';
|
||||
import { useSteerCancel, useLocalize } from '~/hooks';
|
||||
import { cn } from '~/utils';
|
||||
import store from '~/store';
|
||||
|
||||
const splitFiles = (files?: TMessage['files']) => {
|
||||
const images: NonNullable<TMessage['files']> = [];
|
||||
const others: NonNullable<TMessage['files']> = [];
|
||||
for (const file of files ?? []) {
|
||||
(file.type?.startsWith('image/') === true ? images : others).push(file);
|
||||
}
|
||||
return { images, others };
|
||||
};
|
||||
|
||||
/**
|
||||
* One steer on its way into the run, anchored above the composer as a message
|
||||
* bubble rather than a control chip — the words are already part of the
|
||||
|
|
@ -16,6 +27,10 @@ import store from '~/store';
|
|||
* `on_steer_applied`, when the persisted STEER part lands at its authoritative
|
||||
* position in the response.
|
||||
*
|
||||
* Text and attachments render through the same leaves as the applied
|
||||
* `SteerPart` (markdown toggle, file preview) so the words don't reformat the
|
||||
* moment the server injects them.
|
||||
*
|
||||
* `sending` is still awaiting its 202 ACK (no server id yet, so nothing to
|
||||
* cancel); `pending` is acknowledged and waiting on the next tool-batch
|
||||
* boundary.
|
||||
|
|
@ -29,8 +44,16 @@ const InFlightSteer = memo(function InFlightSteer({
|
|||
}) {
|
||||
const localize = useLocalize();
|
||||
const cancelSteer = useSteerCancel(conversationId);
|
||||
const enableUserMsgMarkdown = useRecoilValue<boolean>(store.enableUserMsgMarkdown);
|
||||
const [selectedFile, setSelectedFile] = useState<Partial<TFile> | null>(null);
|
||||
const handlePreviewClose = useCallback((open: boolean) => {
|
||||
if (!open) {
|
||||
setSelectedFile(null);
|
||||
}
|
||||
}, []);
|
||||
|
||||
const { images, others } = useMemo(() => splitFiles(steer.files), [steer.files]);
|
||||
const sending = steer.status === 'sending';
|
||||
const files = steer.files ?? [];
|
||||
|
||||
return (
|
||||
<div
|
||||
|
|
@ -39,33 +62,38 @@ const InFlightSteer = memo(function InFlightSteer({
|
|||
data-steer-status={steer.status}
|
||||
className="group flex flex-col items-start gap-1.5"
|
||||
>
|
||||
{files.length > 0 && (
|
||||
<div className="flex flex-wrap gap-2">
|
||||
{files.map((file) =>
|
||||
file.type?.startsWith('image/') === true ? (
|
||||
<div key={file.file_id} className="h-14 w-14 overflow-hidden rounded-lg">
|
||||
<Image
|
||||
imagePath={file.preview ?? file.filepath ?? ''}
|
||||
height={file.height ?? 1920}
|
||||
width={file.width ?? 1080}
|
||||
altText={file.filename ?? localize('com_ui_attached_image')}
|
||||
/>
|
||||
</div>
|
||||
) : (
|
||||
<FileContainer key={file.file_id} file={file as TFile} />
|
||||
),
|
||||
)}
|
||||
{(images.length > 0 || others.length > 0) && (
|
||||
<div className="flex flex-wrap items-center gap-2">
|
||||
{others.map((file) => (
|
||||
<FileContainer
|
||||
key={file.file_id}
|
||||
file={file as TFile}
|
||||
onClick={() => setSelectedFile(file)}
|
||||
/>
|
||||
))}
|
||||
{images.map((file) => (
|
||||
<div
|
||||
key={file.file_id}
|
||||
className="overflow-hidden rounded-xl border border-border-light"
|
||||
>
|
||||
<ImagePreview
|
||||
url={file.preview ?? file.filepath}
|
||||
alt={file.filename ?? localize('com_ui_attached_image')}
|
||||
/>
|
||||
</div>
|
||||
))}
|
||||
</div>
|
||||
)}
|
||||
<div className="flex max-w-full items-center gap-1.5">
|
||||
<div
|
||||
className={cn(
|
||||
'min-w-0 rounded-3xl bg-surface-secondary px-4 py-2 text-sm text-text-primary',
|
||||
'whitespace-pre-wrap break-words',
|
||||
'markdown prose message-content dark:prose-invert light min-w-0 break-words',
|
||||
'rounded-3xl bg-surface-secondary px-4 py-2 text-sm text-text-primary dark:text-gray-20',
|
||||
!enableUserMsgMarkdown && 'whitespace-pre-wrap',
|
||||
sending && 'opacity-70',
|
||||
)}
|
||||
>
|
||||
{steer.text}
|
||||
{enableUserMsgMarkdown ? <MarkdownLite content={steer.text} /> : steer.text}
|
||||
</div>
|
||||
<Zap
|
||||
className={cn('h-3.5 w-3.5 shrink-0 text-amber-500', sending && 'opacity-50')}
|
||||
|
|
@ -84,6 +112,17 @@ const InFlightSteer = memo(function InFlightSteer({
|
|||
</button>
|
||||
)}
|
||||
</div>
|
||||
{others.length > 0 && (
|
||||
<FilePreviewDialog
|
||||
open={selectedFile !== null}
|
||||
onOpenChange={handlePreviewClose}
|
||||
fileName={selectedFile?.filename ?? ''}
|
||||
fileId={selectedFile?.file_id}
|
||||
filePath={selectedFile?.filepath}
|
||||
fileType={selectedFile?.type ?? undefined}
|
||||
fileSize={(selectedFile as TFile | null)?.bytes}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
});
|
||||
|
|
@ -101,7 +140,7 @@ const InFlightSteers = memo(function InFlightSteers({
|
|||
}) {
|
||||
const localize = useLocalize();
|
||||
const steers = useRecoilValue(store.pendingSteersByConvoId(conversationId));
|
||||
const inFlight = steers.filter((steer) => steer.status !== 'failed');
|
||||
const inFlight = useMemo(() => steers.filter((steer) => steer.status !== 'failed'), [steers]);
|
||||
|
||||
if (inFlight.length === 0) {
|
||||
return null;
|
||||
|
|
|
|||
|
|
@ -18,23 +18,45 @@ jest.mock('~/data-provider', () => ({
|
|||
|
||||
jest.mock('~/components/Chat/Input/Files/FileContainer', () => ({
|
||||
__esModule: true,
|
||||
default: ({ file }: { file: { filename?: string } }) => (
|
||||
<div data-testid="steer-file">{file.filename}</div>
|
||||
default: ({ file, onClick }: { file: { filename?: string }; onClick?: () => void }) => (
|
||||
<button type="button" data-testid="steer-file" onClick={onClick}>
|
||||
{file.filename}
|
||||
</button>
|
||||
),
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Chat/Messages/Content/Image', () => ({
|
||||
/** The composer thumbnail path: a fixed-size button painted with a background
|
||||
* image, not an <img> — assert on the url it was handed. */
|
||||
jest.mock('~/components/Chat/Input/Files/ImagePreview', () => ({
|
||||
__esModule: true,
|
||||
default: ({ altText }: { altText: string }) => <img alt={altText} data-testid="steer-image" />,
|
||||
default: ({ url, alt }: { url?: string; alt?: string }) => (
|
||||
<button type="button" data-testid="steer-image" data-url={url} aria-label={alt} />
|
||||
),
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Chat/Messages/Content/FilePreviewDialog', () => ({
|
||||
__esModule: true,
|
||||
default: ({ open, fileName }: { open: boolean; fileName: string }) =>
|
||||
open ? <div data-testid="steer-file-preview">{fileName}</div> : null,
|
||||
}));
|
||||
|
||||
jest.mock('~/components/Chat/Messages/Content/MarkdownLite', () => ({
|
||||
__esModule: true,
|
||||
default: ({ content }: { content: string }) => (
|
||||
<span data-testid="steer-markdown">{content}</span>
|
||||
),
|
||||
}));
|
||||
|
||||
const CONVO_ID = 'convo-in-flight';
|
||||
|
||||
function renderSteers(steers: PendingSteer[]) {
|
||||
function renderSteers(steers: PendingSteer[], options?: { enableUserMsgMarkdown?: boolean }) {
|
||||
return render(
|
||||
<RecoilRoot
|
||||
initializeState={({ set }) => {
|
||||
set(store.pendingSteersByConvoId(CONVO_ID), steers);
|
||||
if (options?.enableUserMsgMarkdown != null) {
|
||||
set(store.enableUserMsgMarkdown, options.enableUserMsgMarkdown);
|
||||
}
|
||||
}}
|
||||
>
|
||||
<InFlightSteers conversationId={CONVO_ID} />
|
||||
|
|
@ -97,7 +119,7 @@ describe('InFlightSteers', () => {
|
|||
expect(screen.getByText('network flake')).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it('previews image attachments and lists other files', () => {
|
||||
it('renders images through the composer thumbnail path, not the full-size message image', () => {
|
||||
renderSteers([
|
||||
{
|
||||
steerId: 's1',
|
||||
|
|
@ -105,12 +127,65 @@ describe('InFlightSteers', () => {
|
|||
status: 'pending',
|
||||
createdAt: 1,
|
||||
files: [
|
||||
{ file_id: 'f1', filename: 'notes.pdf', type: 'application/pdf' },
|
||||
{ file_id: 'f2', filename: 'shot.png', type: 'image/png', filepath: '/images/shot.png' },
|
||||
],
|
||||
},
|
||||
]);
|
||||
// The message `Image` reserves height from the file's dimensions, so it
|
||||
// cannot be clipped down to a thumbnail; ImagePreview is fixed-size.
|
||||
expect(screen.getByTestId('steer-image')).toHaveAttribute('data-url', '/images/shot.png');
|
||||
});
|
||||
|
||||
it('prefers the local preview url for an image that is still uploading', () => {
|
||||
renderSteers([
|
||||
{
|
||||
steerId: 's1',
|
||||
text: 'see attached',
|
||||
status: 'pending',
|
||||
createdAt: 1,
|
||||
files: [
|
||||
{
|
||||
file_id: 'f2',
|
||||
filename: 'shot.png',
|
||||
type: 'image/png',
|
||||
preview: 'blob:local-preview',
|
||||
filepath: '/images/shot.png',
|
||||
},
|
||||
],
|
||||
},
|
||||
]);
|
||||
expect(screen.getByTestId('steer-image')).toHaveAttribute('data-url', 'blob:local-preview');
|
||||
});
|
||||
|
||||
it('keeps non-image attachments previewable while the steer waits', () => {
|
||||
renderSteers([
|
||||
{
|
||||
steerId: 's1',
|
||||
text: 'see attached',
|
||||
status: 'pending',
|
||||
createdAt: 1,
|
||||
files: [{ file_id: 'f1', filename: 'notes.pdf', type: 'application/pdf' }],
|
||||
},
|
||||
]);
|
||||
expect(screen.getByTestId('steer-file')).toHaveTextContent('notes.pdf');
|
||||
expect(screen.getByTestId('steer-image')).toBeInTheDocument();
|
||||
expect(screen.queryByTestId('steer-file-preview')).toBeNull();
|
||||
|
||||
fireEvent.click(screen.getByTestId('steer-file'));
|
||||
expect(screen.getByTestId('steer-file-preview')).toHaveTextContent('notes.pdf');
|
||||
});
|
||||
|
||||
it('renders markdown the same way the applied part will, so text does not reflow on apply', () => {
|
||||
renderSteers([{ steerId: 's1', text: '**bold** steer', status: 'pending', createdAt: 1 }], {
|
||||
enableUserMsgMarkdown: true,
|
||||
});
|
||||
expect(screen.getByTestId('steer-markdown')).toHaveTextContent('**bold** steer');
|
||||
});
|
||||
|
||||
it('renders raw text when user-message markdown is off', () => {
|
||||
renderSteers([{ steerId: 's1', text: '**bold** steer', status: 'pending', createdAt: 1 }], {
|
||||
enableUserMsgMarkdown: false,
|
||||
});
|
||||
expect(screen.queryByTestId('steer-markdown')).toBeNull();
|
||||
expect(screen.getByText('**bold** steer')).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue