mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-08-03 22:32:42 +00:00
🎯 fix: Preserve Selected Artifact When Clicking Artifact Button (#12601)
* fix: preserve selected artifact when clicking artifact button * fix: preserve artifact selection on click and during streaming * fix: remove broken streaming guard, add JSDoc and test - Remove `userHasManualSelection` guard from effect #3: it cannot distinguish manual clicks from system auto-selection, blocking auto-advancement to new artifacts during streaming. - Add JSDoc on `currentArtifactIdRef` explaining why it must not be added to effect deps (toggle-close regression). - Add test verifying auto-advancement during streaming. * style: use standard multi-line JSDoc format for ref comment --------- Co-authored-by: Danny Avila <danny@librechat.ai>
This commit is contained in:
parent
c4bb41137d
commit
5cc783b8e8
3 changed files with 82 additions and 10 deletions
|
|
@ -63,16 +63,12 @@ const ArtifactButton = ({ artifact }: { artifact: Artifact | null }) => {
|
|||
return;
|
||||
}
|
||||
|
||||
resetCurrentArtifactId();
|
||||
setCurrentArtifactId(artifact.id);
|
||||
setVisible(true);
|
||||
|
||||
if (artifacts?.[artifact.id] == null) {
|
||||
setArtifacts(visibleArtifacts);
|
||||
}
|
||||
|
||||
setTimeout(() => {
|
||||
setCurrentArtifactId(artifact.id);
|
||||
}, 15);
|
||||
};
|
||||
|
||||
const buttonClass = cn(
|
||||
|
|
|
|||
|
|
@ -372,6 +372,75 @@ describe('useArtifacts', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('artifact selection preservation', () => {
|
||||
it('should preserve selection when a new artifact is added', () => {
|
||||
const artifact1 = createArtifact({ id: 'artifact-1', lastUpdateTime: 1000 });
|
||||
|
||||
(useRecoilValue as jest.Mock).mockReturnValue({ 'artifact-1': artifact1 });
|
||||
(useRecoilState as jest.Mock).mockReturnValue(['artifact-1', mockSetCurrentArtifactId]);
|
||||
|
||||
const { rerender } = renderHook(() => useArtifacts());
|
||||
|
||||
mockSetCurrentArtifactId.mockClear();
|
||||
|
||||
/** Append a second artifact; mock still returns 'artifact-1' as current */
|
||||
const artifact2 = createArtifact({ id: 'artifact-2', lastUpdateTime: 2000 });
|
||||
(useRecoilValue as jest.Mock).mockReturnValue({
|
||||
'artifact-1': artifact1,
|
||||
'artifact-2': artifact2,
|
||||
});
|
||||
|
||||
rerender();
|
||||
|
||||
expect(mockSetCurrentArtifactId).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should advance to new artifact during streaming', () => {
|
||||
const artifact1 = createArtifact({ id: 'artifact-1', lastUpdateTime: 1000, content: 'c1' });
|
||||
|
||||
(useRecoilValue as jest.Mock).mockReturnValue({ 'artifact-1': artifact1 });
|
||||
(useRecoilState as jest.Mock).mockReturnValue(['artifact-1', mockSetCurrentArtifactId]);
|
||||
(useArtifactsContext as jest.Mock).mockReturnValue({
|
||||
...defaultContext,
|
||||
isSubmitting: true,
|
||||
latestMessageId: 'msg-1',
|
||||
});
|
||||
|
||||
const { rerender } = renderHook(() => useArtifacts());
|
||||
mockSetCurrentArtifactId.mockClear();
|
||||
|
||||
const artifact2 = createArtifact({ id: 'artifact-2', lastUpdateTime: 2000, content: 'c2' });
|
||||
(useRecoilValue as jest.Mock).mockReturnValue({
|
||||
'artifact-1': artifact1,
|
||||
'artifact-2': artifact2,
|
||||
});
|
||||
|
||||
rerender();
|
||||
|
||||
expect(mockSetCurrentArtifactId).toHaveBeenCalledWith('artifact-2');
|
||||
});
|
||||
|
||||
it('should keep selection null after an explicit reset', () => {
|
||||
const artifact1 = createArtifact({ id: 'artifact-1', lastUpdateTime: 1000 });
|
||||
|
||||
/** First render: valid selection */
|
||||
(useRecoilValue as jest.Mock).mockReturnValue({ 'artifact-1': artifact1 });
|
||||
(useRecoilState as jest.Mock).mockReturnValue(['artifact-1', mockSetCurrentArtifactId]);
|
||||
|
||||
const { rerender } = renderHook(() => useArtifacts());
|
||||
|
||||
mockSetCurrentArtifactId.mockClear();
|
||||
|
||||
/** Rerender: currentArtifactId transitions to null (user closed the panel) */
|
||||
(useRecoilState as jest.Mock).mockReturnValue([null, mockSetCurrentArtifactId]);
|
||||
|
||||
rerender();
|
||||
|
||||
/** Should NOT bounce back to 'artifact-1' */
|
||||
expect(mockSetCurrentArtifactId).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('cleanup on unmount', () => {
|
||||
it('should reset artifacts when unmounting', () => {
|
||||
(useRecoilValue as jest.Mock).mockReturnValue({});
|
||||
|
|
|
|||
|
|
@ -51,12 +51,19 @@ export default function useArtifacts() {
|
|||
};
|
||||
}, [conversationId, resetArtifacts, resetCurrentArtifactId]);
|
||||
|
||||
/**
|
||||
* Read currentArtifactId in effects without subscribing as a dependency.
|
||||
* Adding it to effect deps fires auto-select on every reset, breaking toggle-close.
|
||||
*/
|
||||
const currentArtifactIdRef = useRef(currentArtifactId);
|
||||
currentArtifactIdRef.current = currentArtifactId;
|
||||
|
||||
useEffect(() => {
|
||||
if (orderedArtifactIds.length > 0) {
|
||||
const latestArtifactId = orderedArtifactIds[orderedArtifactIds.length - 1];
|
||||
setCurrentArtifactId(latestArtifactId);
|
||||
}
|
||||
}, [setCurrentArtifactId, orderedArtifactIds]);
|
||||
if (orderedArtifactIds.length === 0) return;
|
||||
const currentId = currentArtifactIdRef.current;
|
||||
if (currentId != null && orderedArtifactIds.includes(currentId)) return;
|
||||
setCurrentArtifactId(orderedArtifactIds[orderedArtifactIds.length - 1]);
|
||||
}, [orderedArtifactIds, setCurrentArtifactId]);
|
||||
|
||||
/**
|
||||
* Manage artifact selection and code tab switching for non-enclosed artifacts
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue