🔖 fix: Remove Local State from Bookmark Menu (#5181)

* chore: remove redundant * fix: bookmark menu statefulness by removing local state
danny-avila · Jan 4, 2025 · 766657d · 766657d
1 parent 7c61115
commit 766657d
Show file tree

Hide file tree

Showing 5 changed files with 24 additions and 26 deletions.
diff --git a/client/src/components/Chat/Input/HeaderOptions.tsx b/client/src/components/Chat/Input/HeaderOptions.tsx
@@ -2,7 +2,7 @@ import { useRecoilState } from 'recoil';
 import { Settings2 } from 'lucide-react';
 import { Root, Anchor } from '@radix-ui/react-popover';
 import { useState, useEffect, useMemo } from 'react';
-import { tPresetUpdateSchema, EModelEndpoint, isParamEndpoint } from 'librechat-data-provider';
+import { tConvoUpdateSchema, EModelEndpoint, isParamEndpoint } from 'librechat-data-provider';
 import type { TPreset, TInterfaceConfig } from 'librechat-data-provider';
 import { EndpointSettings, SaveAsPresetDialog, AlternativeSettings } from '~/components/Endpoints';
 import { PluginStoreDialog, TooltipAnchor } from '~/components';
@@ -123,7 +123,7 @@ export default function HeaderOptions({
                 open={saveAsDialogShow}
                 onOpenChange={setSaveAsDialogShow}
                 preset={
-                  tPresetUpdateSchema.parse({
+                  tConvoUpdateSchema.parse({
                     ...conversation,
                   }) as TPreset
                 }

diff --git a/client/src/components/Chat/Menus/BookmarkMenu.tsx b/client/src/components/Chat/Menus/BookmarkMenu.tsx
@@ -27,15 +27,14 @@ const BookmarkMenu: FC = () => {
   const conversation = useRecoilValue(store.conversationByIndex(0)) || undefined;
   const conversationId = conversation?.conversationId ?? '';
   const updateConvoTags = useBookmarkSuccess(conversationId);
+  const tags = conversation?.tags;
 
   const menuId = useId();
   const [isMenuOpen, setIsMenuOpen] = useState(false);
   const [isDialogOpen, setIsDialogOpen] = useState(false);
-  const [tags, setTags] = useState<string[]>(conversation?.tags || []);
 
   const mutation = useTagConversationMutation(conversationId, {
     onSuccess: (newTags: string[], vars) => {
-      setTags(newTags);
       updateConvoTags(newTags);
       const tagElement = document.getElementById(vars.tag);
       console.log('tagElement', tagElement);
@@ -82,12 +81,13 @@ const BookmarkMenu: FC = () => {
       const allTags =
         queryClient.getQueryData<TConversationTag[]>([QueryKeys.conversationTags]) ?? [];
       const existingTags = allTags.map((t) => t.tag);
-      const filteredTags = tags.filter((t) => existingTags.includes(t));
+      const filteredTags = tags?.filter((t) => existingTags.includes(t));
 
       logger.log('tag_mutation', 'BookmarkMenu - handleSubmit: tags after filtering', filteredTags);
-      const newTags = filteredTags.includes(tag)
-        ? filteredTags.filter((t) => t !== tag)
-        : [...filteredTags, tag];
+      const newTags =
+        filteredTags?.includes(tag) === true
+          ? filteredTags.filter((t) => t !== tag)
+          : [...(filteredTags ?? []), tag];
 
       logger.log('tag_mutation', 'BookmarkMenu - handleSubmit: tags after', newTags);
       mutation.mutate({
@@ -115,16 +115,17 @@ const BookmarkMenu: FC = () => {
 
     if (data) {
       for (const tag of data) {
-        const isSelected = tags.includes(tag.tag);
+        const isSelected = tags?.includes(tag.tag);
         items.push({
           id: tag.tag,
-          hideOnClick: false,
           label: tag.tag,
-          icon: isSelected ? (
-            <BookmarkFilledIcon className="size-4" />
-          ) : (
-            <BookmarkIcon className="size-4" />
-          ),
+          hideOnClick: false,
+          icon:
+            isSelected === true ? (
+              <BookmarkFilledIcon className="size-4" />
+            ) : (
+              <BookmarkIcon className="size-4" />
+            ),
           onClick: () => handleSubmit(tag.tag),
           disabled: mutation.isLoading,
         });
@@ -142,7 +143,7 @@ const BookmarkMenu: FC = () => {
     if (mutation.isLoading) {
       return <Spinner aria-label="Spinner" />;
     }
-    if (tags.length > 0) {
+    if ((tags?.length ?? 0) > 0) {
       return <BookmarkFilledIcon className="icon-sm" aria-label="Filled Bookmark" />;
     }
     return <BookmarkIcon className="icon-sm" aria-label="Bookmark" />;
@@ -155,6 +156,7 @@ const BookmarkMenu: FC = () => {
         menuId={menuId}
         isOpen={isMenuOpen}
         setIsOpen={setIsMenuOpen}
+        keyPrefix={`${conversationId}-bookmark-`}
         trigger={
           <TooltipAnchor
             description={localize('com_ui_bookmarks_add')}
@@ -177,8 +179,8 @@ const BookmarkMenu: FC = () => {
       />
       <BookmarkEditDialog
         tags={tags}
-        setTags={setTags}
         open={isDialogOpen}
+        setTags={updateConvoTags}
         setOpen={setIsDialogOpen}
         triggerRef={newBookmarkRef}
         conversationId={conversationId}

diff --git a/client/src/components/SidePanel/Parameters/Panel.tsx b/client/src/components/SidePanel/Parameters/Panel.tsx
@@ -1,6 +1,6 @@
 import React, { useMemo, useState, useEffect, useCallback } from 'react';
 import { useGetEndpointsQuery } from 'librechat-data-provider/react-query';
-import { getSettingsKeys, tPresetUpdateSchema } from 'librechat-data-provider';
+import { getSettingsKeys, tConvoUpdateSchema } from 'librechat-data-provider';
 import type { TPreset } from 'librechat-data-provider';
 import { SaveAsPresetDialog } from '~/components/Endpoints';
 import { useSetIndexOptions, useLocalize } from '~/hooks';
@@ -106,7 +106,7 @@ export default function Parameters() {
   }, [parameters, setConversation]);
 
   const openDialog = useCallback(() => {
-    const newPreset = tPresetUpdateSchema.parse({
+    const newPreset = tConvoUpdateSchema.parse({
       ...conversation,
     }) as TPreset;
     setPreset(newPreset);

diff --git a/client/src/components/ui/DropdownPopup.tsx b/client/src/components/ui/DropdownPopup.tsx
@@ -4,6 +4,7 @@ import type * as t from '~/common';
 import { cn } from '~/utils';
 
 interface DropdownProps {
+  keyPrefix?: string;
   trigger: React.ReactNode;
   items: t.MenuItemProps[];
   isOpen: boolean;
@@ -20,6 +21,7 @@ interface DropdownProps {
 }
 
 const DropdownPopup: React.FC<DropdownProps> = ({
+  keyPrefix,
   trigger,
   items,
   isOpen,
@@ -53,7 +55,7 @@ const DropdownPopup: React.FC<DropdownProps> = ({
             }
             return (
               <Ariakit.MenuItem
-                key={index}
+                key={`${keyPrefix ?? ''}${index}`}
                 id={item.id}
                 className={cn(
                   'group flex w-full cursor-pointer items-center gap-2 rounded-lg px-3 py-3.5 text-sm text-text-primary outline-none transition-colors duration-200 hover:bg-surface-hover focus:bg-surface-hover md:px-2.5 md:py-2',

diff --git a/packages/data-provider/src/schemas.ts b/packages/data-provider/src/schemas.ts
@@ -630,12 +630,6 @@ export const tConvoUpdateSchema = tConversationSchema.merge(
   }),
 );
 
-export const tPresetUpdateSchema = tConversationSchema.merge(
-  z.object({
-    endpoint: extendedModelEndpointSchema.nullable(),
-  }),
-);
-
 export type TPreset = z.infer<typeof tPresetSchema>;
 
 export type TSetOption = (