🔄 refactor: Artifact Visibility Management (#7181)

* fix: Reset artifacts on unmount and remove useIdChangeEffect hook

* feat: Replace SVG icons with Lucide icons for improved consistency

* fix: Refactor artifact reset logic on unmount and conversation change

* refactor: Rename artifactsVisible to artifactsVisibility for consistency

* feat: Replace custom SVG icons with Lucide icons for improved consistency

* feat: Add visibleArtifacts atom for managing visibility state

* feat: Implement debounced visibility state management for artifacts

* refactor: Add useIdChangeEffect hook to reset visible artifacts on conversation ID change

* refactor: Remove unnecessary dependency from useMemo in TextPart component

* refactor: Enhance artifact visibility management by incorporating location checks for search path

* refactor: Improve transition effects for artifact visibility in Artifacts component

* chore: Remove preprocessCodeArtifacts function and related tests

* fix: Update regex for detecting enclosed artifacts in latest message

* refactor: Update artifact visibility checks to be more generic (not just search)

* chore: Enhance artifact visibility logging

* refactor: Extract closeArtifacts function to improve button click handling

* refactor: remove nested logic from use artifacts effect

* refactor: Update regex for detecting enclosed artifacts to handle new line variations
This commit is contained in:
Danny Avila 2025-05-01 14:40:39 -04:00 committed by GitHub
parent e6e7935fd8
commit 9a7f763714
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 132 additions and 209 deletions

View file

@ -2,6 +2,7 @@ import React, { useEffect, useCallback, useRef, useState } from 'react';
import throttle from 'lodash/throttle';
import { visit } from 'unist-util-visit';
import { useSetRecoilState } from 'recoil';
import { useLocation } from 'react-router-dom';
import type { Pluggable } from 'unified';
import type { Artifact } from '~/common';
import { useMessageContext, useArtifactContext } from '~/Providers';
@ -45,6 +46,7 @@ export function Artifact({
children: React.ReactNode | { props: { children: React.ReactNode } };
node: unknown;
}) {
const location = useLocation();
const { messageId } = useMessageContext();
const { getNextIndex, resetCounter } = useArtifactContext();
const artifactIndex = useRef(getNextIndex(false)).current;
@ -86,6 +88,10 @@ export function Artifact({
lastUpdateTime: now,
};
if (!location.pathname.includes('/c/')) {
return setArtifact(currentArtifact);
}
setArtifacts((prevArtifacts) => {
if (
prevArtifacts?.[artifactKey] != null &&
@ -110,6 +116,7 @@ export function Artifact({
props.identifier,
messageId,
artifactIndex,
location.pathname,
]);
useEffect(() => {

View file

@ -1,15 +1,52 @@
import { useSetRecoilState, useResetRecoilState } from 'recoil';
import { useEffect, useRef } from 'react';
import debounce from 'lodash/debounce';
import { useLocation } from 'react-router-dom';
import { useRecoilState, useSetRecoilState, useResetRecoilState } from 'recoil';
import type { Artifact } from '~/common';
import FilePreview from '~/components/Chat/Input/Files/FilePreview';
import { getFileType, logger } from '~/utils';
import { useLocalize } from '~/hooks';
import { getFileType } from '~/utils';
import store from '~/store';
const ArtifactButton = ({ artifact }: { artifact: Artifact | null }) => {
const localize = useLocalize();
const setVisible = useSetRecoilState(store.artifactsVisible);
const location = useLocation();
const setVisible = useSetRecoilState(store.artifactsVisibility);
const [artifacts, setArtifacts] = useRecoilState(store.artifactsState);
const setCurrentArtifactId = useSetRecoilState(store.currentArtifactId);
const resetCurrentArtifactId = useResetRecoilState(store.currentArtifactId);
const [visibleArtifacts, setVisibleArtifacts] = useRecoilState(store.visibleArtifacts);
const debouncedSetVisibleRef = useRef(
debounce((artifactToSet: Artifact) => {
logger.log(
'artifacts_visibility',
'Setting artifact to visible state from Artifact button',
artifactToSet,
);
setVisibleArtifacts((prev) => ({
...prev,
[artifactToSet.id]: artifactToSet,
}));
}, 750),
);
useEffect(() => {
if (artifact == null || artifact?.id == null || artifact.id === '') {
return;
}
if (!location.pathname.includes('/c/')) {
return;
}
const debouncedSetVisible = debouncedSetVisibleRef.current;
debouncedSetVisible(artifact);
return () => {
debouncedSetVisible.cancel();
};
}, [artifact, location.pathname]);
if (artifact === null || artifact === undefined) {
return null;
}
@ -20,8 +57,14 @@ const ArtifactButton = ({ artifact }: { artifact: Artifact | null }) => {
<button
type="button"
onClick={() => {
if (!location.pathname.includes('/c/')) {
return;
}
resetCurrentArtifactId();
setVisible(true);
if (artifacts?.[artifact.id] == null) {
setArtifacts(visibleArtifacts);
}
setTimeout(() => {
setCurrentArtifactId(artifact.id);
}, 15);

View file

@ -1,7 +1,7 @@
import { useRef, useState, useEffect } from 'react';
import { RefreshCw } from 'lucide-react';
import { useSetRecoilState } from 'recoil';
import * as Tabs from '@radix-ui/react-tabs';
import { ArrowLeft, ChevronLeft, ChevronRight, RefreshCw, X } from 'lucide-react';
import type { SandpackPreviewRef, CodeEditorRef } from '@codesandbox/sandpack-react';
import useArtifacts from '~/hooks/Artifacts/useArtifacts';
import DownloadArtifact from './DownloadArtifact';
@ -18,7 +18,7 @@ export default function Artifacts() {
const previewRef = useRef<SandpackPreviewRef>();
const [isVisible, setIsVisible] = useState(false);
const [isRefreshing, setIsRefreshing] = useState(false);
const setArtifactsVisible = useSetRecoilState(store.artifactsVisible);
const setArtifactsVisible = useSetRecoilState(store.artifactsVisibility);
useEffect(() => {
setIsVisible(true);
@ -48,37 +48,26 @@ export default function Artifacts() {
setTimeout(() => setIsRefreshing(false), 750);
};
const closeArtifacts = () => {
setIsVisible(false);
setTimeout(() => setArtifactsVisible(false), 300);
};
return (
<Tabs.Root value={activeTab} onValueChange={setActiveTab} asChild>
{/* Main Parent */}
<div className="flex h-full w-full items-center justify-center">
{/* Main Container */}
<div
className={`flex h-full w-full flex-col overflow-hidden border border-border-medium bg-surface-primary text-xl text-text-primary shadow-xl transition-all duration-300 ease-in-out ${
isVisible
? 'translate-x-0 scale-100 opacity-100'
: 'translate-x-full scale-95 opacity-0'
className={`flex h-full w-full flex-col overflow-hidden border border-border-medium bg-surface-primary text-xl text-text-primary shadow-xl transition-all duration-500 ease-in-out ${
isVisible ? 'scale-100 opacity-100 blur-0' : 'scale-105 opacity-0 blur-sm'
}`}
>
{/* Header */}
<div className="flex items-center justify-between border-b border-border-medium bg-surface-primary-alt p-2">
<div className="flex items-center">
<button
className="mr-2 text-text-secondary"
onClick={() => {
setIsVisible(false);
setTimeout(() => setArtifactsVisible(false), 300);
}}
>
<svg
xmlns="http://www.w3.org/2000/svg"
width="16"
height="16"
fill="currentColor"
viewBox="0 0 256 256"
>
<path d="M224,128a8,8,0,0,1-8,8H59.31l58.35,58.34a8,8,0,0,1-11.32,11.32l-72-72a8,8,0,0,1,0-11.32l72-72a8,8,0,0,1,11.32,11.32L59.31,120H216A8,8,0,0,1,224,128Z" />
</svg>
<button className="mr-2 text-text-secondary" onClick={closeArtifacts}>
<ArrowLeft className="h-4 w-4" />
</button>
<h3 className="truncate text-sm text-text-primary">{currentArtifact.title}</h3>
</div>
@ -118,22 +107,8 @@ export default function Artifacts() {
{localize('com_ui_code')}
</Tabs.Trigger>
</Tabs.List>
<button
className="text-text-secondary"
onClick={() => {
setIsVisible(false);
setTimeout(() => setArtifactsVisible(false), 300);
}}
>
<svg
xmlns="http://www.w3.org/2000/svg"
width="16"
height="16"
fill="currentColor"
viewBox="0 0 256 256"
>
<path d="M205.66,194.34a8,8,0,0,1-11.32,11.32L128,139.31,61.66,205.66a8,8,0,0,1-11.32-11.32L116.69,128,50.34,61.66A8,8,0,0,1,61.66,50.34L128,116.69l66.34-66.35a8,8,0,0,1,11.32,11.32L139.31,128Z" />
</svg>
<button className="text-text-secondary" onClick={closeArtifacts}>
<X className="h-4 w-4" />
</button>
</div>
</div>
@ -149,29 +124,13 @@ export default function Artifacts() {
<div className="flex items-center justify-between border-t border-border-medium bg-surface-primary-alt p-2 text-sm text-text-secondary">
<div className="flex items-center">
<button onClick={() => cycleArtifact('prev')} className="mr-2 text-text-secondary">
<svg
xmlns="http://www.w3.org/2000/svg"
width="16"
height="16"
fill="currentColor"
viewBox="0 0 256 256"
>
<path d="M165.66,202.34a8,8,0,0,1-11.32,11.32l-80-80a8,8,0,0,1,0-11.32l80-80a8,8,0,0,1,11.32,11.32L91.31,128Z" />
</svg>
<ChevronLeft className="h-4 w-4" />
</button>
<span className="text-xs">{`${currentIndex + 1} / ${
orderedArtifactIds.length
}`}</span>
<button onClick={() => cycleArtifact('next')} className="ml-2 text-text-secondary">
<svg
xmlns="http://www.w3.org/2000/svg"
width="16"
height="16"
fill="currentColor"
viewBox="0 0 256 256"
>
<path d="M181.66,133.66l-80,80a8,8,0,0,1-11.32-11.32L164.69,128,90.34,53.66a8,8,0,0,1,11.32-11.32l80,80A8,8,0,0,1,181.66,133.66Z" />
</svg>
<ChevronRight className="h-4 w-4" />
</button>
</div>
<div className="flex items-center gap-2">

View file

@ -35,7 +35,7 @@ const TextPart = memo(({ text, isCreatedByUser, showCursor }: TextPartProps) =>
} else {
return <>{text}</>;
}
}, [isCreatedByUser, enableUserMsgMarkdown, text, showCursorState, isLatestMessage]);
}, [isCreatedByUser, enableUserMsgMarkdown, text, isLatestMessage]);
return (
<div

View file

@ -12,7 +12,7 @@ import store from '~/store';
export default function Presentation({ children }: { children: React.ReactNode }) {
const artifacts = useRecoilValue(store.artifactsState);
const artifactsVisible = useRecoilValue(store.artifactsVisible);
const artifactsVisibility = useRecoilValue(store.artifactsVisibility);
const setFilesToDelete = useSetFilesToDelete();
@ -64,7 +64,7 @@ export default function Presentation({ children }: { children: React.ReactNode }
fullPanelCollapse={fullCollapse}
defaultCollapsed={defaultCollapsed}
artifacts={
artifactsVisible === true && Object.keys(artifacts ?? {}).length > 0 ? (
artifactsVisibility === true && Object.keys(artifacts ?? {}).length > 0 ? (
<EditorProvider>
<Artifacts />
</EditorProvider>

View file

@ -1,9 +1,9 @@
import { useMemo, useState, useEffect, useRef } from 'react';
import { Constants } from 'librechat-data-provider';
import { useRecoilState, useRecoilValue, useResetRecoilState } from 'recoil';
import { getLatestText, logger } from '~/utils';
import { useChatContext } from '~/Providers';
import { getKey } from '~/utils/artifacts';
import { getLatestText } from '~/utils';
import store from '~/store';
export default function useArtifacts() {
@ -37,16 +37,20 @@ export default function useArtifacts() {
hasEnclosedArtifactRef.current = false;
};
if (
conversation &&
conversation.conversationId !== prevConversationIdRef.current &&
conversation?.conversationId !== prevConversationIdRef.current &&
prevConversationIdRef.current != null
) {
resetState();
} else if (conversation && conversation.conversationId === Constants.NEW_CONVO) {
} else if (conversation?.conversationId === Constants.NEW_CONVO) {
resetState();
}
prevConversationIdRef.current = conversation?.conversationId ?? null;
}, [conversation, resetArtifacts, resetCurrentArtifactId]);
/** Resets artifacts when unmounting */
return () => {
logger.log('artifacts_visibility', 'Unmounting artifacts');
resetState();
};
}, [conversation?.conversationId, resetArtifacts, resetCurrentArtifactId]);
useEffect(() => {
if (orderedArtifactIds.length > 0) {
@ -56,16 +60,27 @@ export default function useArtifacts() {
}, [setCurrentArtifactId, orderedArtifactIds]);
useEffect(() => {
if (isSubmitting && orderedArtifactIds.length > 0 && latestMessage) {
if (!isSubmitting) {
return;
}
if (orderedArtifactIds.length === 0) {
return;
}
if (latestMessage == null) {
return;
}
const latestArtifactId = orderedArtifactIds[orderedArtifactIds.length - 1];
const latestArtifact = artifacts?.[latestArtifactId];
if (latestArtifact?.content === lastContentRef.current) {
return;
}
if (latestArtifact?.content !== lastContentRef.current) {
setCurrentArtifactId(latestArtifactId);
lastContentRef.current = latestArtifact?.content ?? null;
const latestMessageText = getLatestText(latestMessage);
const hasEnclosedArtifact = /:::artifact[\s\S]*?(```|:::)\s*$/.test(
const hasEnclosedArtifact =
/:::artifact(?:\{[^}]*\})?(?:\s|\n)*(?:```[\s\S]*?```(?:\s|\n)*)?:::/m.test(
latestMessageText.trim(),
);
@ -80,8 +95,6 @@ export default function useArtifacts() {
hasAutoSwitchedToCodeRef.current = true;
}
}
}
}
}, [setCurrentArtifactId, isSubmitting, orderedArtifactIds, artifacts, latestMessage]);
useEffect(() => {

View file

@ -4,18 +4,18 @@ import { logger } from '~/utils';
import store from '~/store';
/**
* Hook to reset artifacts when the conversation ID changes
* Hook to reset visible artifacts when the conversation ID changes
* @param conversationId - The current conversation ID
*/
export default function useIdChangeEffect(conversationId: string) {
const lastConvoId = useRef<string | null>(null);
const resetArtifacts = useResetRecoilState(store.artifactsState);
const resetVisibleArtifacts = useResetRecoilState(store.visibleArtifacts);
useEffect(() => {
if (conversationId !== lastConvoId.current) {
logger.log('conversation', 'Conversation ID change');
resetArtifacts();
resetVisibleArtifacts();
}
lastConvoId.current = conversationId;
}, [conversationId, resetArtifacts]);
}, [conversationId, resetVisibleArtifacts]);
}

View file

@ -32,13 +32,28 @@ export const currentArtifactId = atom<string | null>({
] as const,
});
export const artifactsVisible = atom<boolean>({
key: 'artifactsVisible',
export const artifactsVisibility = atom<boolean>({
key: 'artifactsVisibility',
default: true,
effects: [
({ onSet, node }) => {
onSet(async (newValue) => {
logger.log('artifacts', 'Recoil Effect: Setting artifactsVisible', {
logger.log('artifacts', 'Recoil Effect: Setting artifactsVisibility', {
key: node.key,
newValue,
});
});
},
] as const,
});
export const visibleArtifacts = atom<Record<string, Artifact | undefined> | null>({
key: 'visibleArtifacts',
default: null,
effects: [
({ onSet, node }) => {
onSet(async (newValue) => {
logger.log('artifacts', 'Recoil Effect: Setting `visibleArtifacts`', {
key: node.key,
newValue,
});

View file

@ -1,94 +0,0 @@
import { preprocessCodeArtifacts } from './artifacts';
describe('preprocessCodeArtifacts', () => {
test('should return non-string inputs unchanged', () => {
expect(preprocessCodeArtifacts(123 as unknown as string)).toBe('');
expect(preprocessCodeArtifacts(null as unknown as string)).toBe('');
expect(preprocessCodeArtifacts(undefined)).toBe('');
expect(preprocessCodeArtifacts({} as unknown as string)).toEqual('');
});
test('should remove <thinking> tags and their content', () => {
const input = '<thinking>This should be removed</thinking>Some content';
const expected = 'Some content';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should remove unclosed <thinking> tags and their content', () => {
const input = '<thinking>This should be removed\nSome content';
const expected = '';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should remove artifact headers up to and including empty code block', () => {
const input = ':::artifact{identifier="test"}\n```\n```\nSome content';
const expected = ':::artifact{identifier="test"}\n```\n```\nSome content';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should keep artifact headers when followed by empty code block and content', () => {
const input = ':::artifact{identifier="test"}\n```\n```\nSome content';
const expected = ':::artifact{identifier="test"}\n```\n```\nSome content';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should handle multiple artifact headers correctly', () => {
const input = ':::artifact{id="1"}\n```\n```\n:::artifact{id="2"}\n```\ncode\n```\nContent';
const expected = ':::artifact{id="1"}\n```\n```\n:::artifact{id="2"}\n```\ncode\n```\nContent';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should handle complex input with multiple patterns', () => {
const input = `
<thinking>Remove this</thinking>
Some text
:::artifact{id="1"}
\`\`\`
\`\`\`
<thinking>And this</thinking>
:::artifact{id="2"}
\`\`\`
keep this code
\`\`\`
More text
`;
const expected = `
Some text
:::artifact{id="1"}
\`\`\`
\`\`\`
:::artifact{id="2"}
\`\`\`
keep this code
\`\`\`
More text
`;
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should remove artifact headers without code blocks', () => {
const input = ':::artifact{identifier="test"}\nSome content without code block';
const expected = '';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should remove artifact headers up to incomplete code block', () => {
const input = ':::artifact{identifier="react-cal';
const expected = '';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should keep artifact headers when any character follows code block', () => {
const input = ':::artifact{identifier="react-calculator"}\n```t';
const expected = ':::artifact{identifier="react-calculator"}\n```t';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
test('should keep artifact headers when whitespace follows code block', () => {
const input = ':::artifact{identifier="react-calculator"}\n``` ';
const expected = ':::artifact{identifier="react-calculator"}\n``` ';
expect(preprocessCodeArtifacts(input)).toBe(expected);
});
});

View file

@ -214,23 +214,3 @@ export const sharedFiles = {
</html>
`,
};
export function preprocessCodeArtifacts(text?: string): string {
if (typeof text !== 'string') {
return '';
}
// Remove <thinking> tags and their content
text = text.replace(/<thinking>[\s\S]*?<\/thinking>|<thinking>[\s\S]*/g, '');
// Process artifact headers
const regex = /(^|\n)(:::artifact[\s\S]*?(?:```[\s\S]*?```|$))/g;
return text.replace(regex, (match, newline, artifactBlock) => {
if (artifactBlock.includes('```') === true) {
// Keep artifact headers with code blocks (empty or not)
return newline + artifactBlock;
}
// Remove artifact headers without code blocks, but keep the newline
return newline;
});
}