fix: solve infinite rerender bug (#241)

* [UI] fix: require namespaces before closing the namespace dialog (#239) * fix: add strict jinja templating * fix: tests had an error * fix: force a non-null namespace * fix: more helpful error message * fix: solve infinite rernder bug
ucbepic · Dec 11, 2024 · e4689e0 · e4689e0
1 parent f701701
commit e4689e0
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 25 deletions.
diff --git a/website/src/components/BookmarksPanel.tsx b/website/src/components/BookmarksPanel.tsx
@@ -26,6 +26,12 @@ import {
   PopoverTrigger,
 } from "@/components/ui/popover";
 import { UserNote } from "@/app/types";
+import {
+  Tooltip,
+  TooltipContent,
+  TooltipProvider,
+  TooltipTrigger,
+} from "@/components/ui/tooltip";
 
 const BookmarksPanel: React.FC = () => {
   const { bookmarks, removeBookmark } = useBookmarkContext();
@@ -96,15 +102,23 @@ const BookmarksPanel: React.FC = () => {
           <Bookmark className="mr-2" size={14} />
           NOTES
         </h2>
-        <Button
-          variant="outline"
-          size="sm"
-          onClick={handleClearAll}
-          className="text-gray-500 hover:text-gray-700"
-        >
-          <X size={14} className="mr-1.5" />
-          Clear All
-        </Button>
+        <TooltipProvider>
+          <Tooltip>
+            <TooltipTrigger asChild>
+              <Button
+                variant="outline"
+                size="icon"
+                onClick={handleClearAll}
+                className="h-8 w-8 text-gray-500 hover:text-gray-700"
+              >
+                <X size={14} />
+              </Button>
+            </TooltipTrigger>
+            <TooltipContent>
+              <p>Clear all notes</p>
+            </TooltipContent>
+          </Tooltip>
+        </TooltipProvider>
       </div>
       <div className="text-xs mb-2 bg-muted/50 p-2 rounded-md">
         <span className="text-muted-foreground font-medium">Tip: </span>

diff --git a/website/src/components/FileExplorer.tsx b/website/src/components/FileExplorer.tsx
@@ -550,9 +550,8 @@ export const FileExplorer: React.FC<FileExplorerProps> = ({
         </h2>
         <DropdownMenu>
           <DropdownMenuTrigger asChild>
-            <Button variant="outline" size="sm">
-              <Upload size={14} className="mr-1.5" />
-              Upload
+            <Button variant="outline" size="icon" className="h-8 w-8">
+              <Upload size={14} />
             </Button>
           </DropdownMenuTrigger>
           <DropdownMenuContent align="end">

diff --git a/website/src/components/NamespaceDialog.tsx b/website/src/components/NamespaceDialog.tsx
@@ -42,12 +42,15 @@ export function NamespaceDialog({
 
     const trimmedNamespace = namespace.trim();
 
-    if (!trimmedNamespace) {
+    if (!trimmedNamespace || trimmedNamespace.length === 0) {
       toast({
         title: "Invalid Namespace",
-        description: "Namespace cannot be empty",
+        description:
+          "Namespace cannot be empty. Please enter a valid namespace.",
         variant: "destructive",
       });
+      setShake(true);
+      setTimeout(() => setShake(false), 500);
       return;
     }
 
@@ -71,6 +74,7 @@ export function NamespaceDialog({
         onSave(trimmedNamespace);
         setShowWarning(false);
         setShake(false);
+        window.location.reload();
       }
     } catch (error) {
       console.error("Error checking namespace:", error);
@@ -86,10 +90,24 @@ export function NamespaceDialog({
 
   const hasNamespaceChanged =
     namespace.trim() !== (currentNamespace || "").trim() &&
-    namespace.trim() !== "";
+    namespace.trim().length > 0;
+
+  const handleOpenChange = (newOpen: boolean) => {
+    if (!newOpen && !currentNamespace?.trim()) {
+      toast({
+        title: "Namespace Required",
+        description: "Please set a namespace before continuing.",
+        variant: "destructive",
+      });
+      setShake(true);
+      setTimeout(() => setShake(false), 500);
+      return;
+    }
+    onOpenChange(newOpen);
+  };
 
   return (
-    <Dialog open={open} onOpenChange={onOpenChange}>
+    <Dialog open={open} onOpenChange={handleOpenChange}>
       <DialogContent className={cn("sm:max-w-lg", shake && "animate-shake")}>
         <DialogHeader>
           <div className="flex items-center gap-2">
@@ -109,24 +127,30 @@ export function NamespaceDialog({
         <div className="py-2">
           <div className="space-y-2">
             <Label htmlFor="namespace" className="text-sm font-medium">
-              Namespace
+              Namespace <span className="text-red-500">*</span>
             </Label>
             <Input
               id="namespace"
-              placeholder="default"
+              placeholder="Enter namespace (e.g., johndoe)"
               value={namespace}
               onChange={(e) => {
                 setNamespace(e.target.value);
                 setShowWarning(false);
               }}
               onBlur={(e) => setNamespace(e.target.value.trim())}
-              className={`w-full ${isChecking ? "border-red-500" : ""}`}
+              className={cn(
+                "w-full",
+                isChecking ? "border-red-500" : "",
+                !namespace.trim() && "border-red-300"
+              )}
+              required
             />
             {isChecking && <p className="text-xs text-red-500">Checking...</p>}
             {showWarning && (
               <div className="text-sm text-orange-700 dark:text-orange-200 bg-orange-100 dark:bg-orange-950 border border-orange-300 dark:border-orange-800 rounded-md p-2 font-medium">
-                Warning: This namespace already exists. Saving will overwrite
-                existing configurations.
+                Warning: This namespace already exists. Setting this namespace
+                may overwrite another user's existing caches, but feel free to
+                ignore this message if this is your namespace.
               </div>
             )}
             <p className="text-xs text-muted-foreground">
@@ -136,7 +160,11 @@ export function NamespaceDialog({
           </div>
         </div>
         <DialogFooter className="flex justify-end gap-2">
-          <Button variant="outline" onClick={() => onOpenChange(false)}>
+          <Button
+            variant="outline"
+            onClick={() => handleOpenChange(false)}
+            disabled={!currentNamespace?.trim()}
+          >
             Cancel
           </Button>
           <Button

diff --git a/website/src/components/Output.tsx b/website/src/components/Output.tsx
@@ -564,7 +564,7 @@ export const Output = memo(() => {
   // Add back the data fetching effect
   useEffect(() => {
     const fetchData = async () => {
-      if (output) {
+      if (output && !isLoadingOutputs) {
         const importantColumns =
           operation?.otherKwargs?.prompts?.[0]?.output_keys;
         try {

diff --git a/website/src/contexts/PipelineContext.tsx b/website/src/contexts/PipelineContext.tsx
@@ -441,8 +441,7 @@ export const PipelineProvider: React.FC<{ children: React.ReactNode }> = ({
               localStorageKeys.NAMESPACE_KEY,
               JSON.stringify(newValue)
             );
-            window.location.reload();
-            return prevState;
+            return { ...prevState, [key]: newValue };
           } else {
             setUnsavedChanges(true);
             return { ...prevState, [key]: newValue };