From 3c600659e51f858013bd987475d6a4bf40835111 Mon Sep 17 00:00:00 2001
From: Matt <mattskent@gmail.com>
Date: Wed, 15 Jan 2025 16:07:06 -0500
Subject: [PATCH] Fix column selector popover behavior, improve product filters

---
 .../components/products/ProductFilters.tsx    | 313 +++++++++++-------
 inventory/src/components/ui/dialog.tsx        |   2 +
 inventory/src/pages/Products.tsx              | 115 ++++---
 3 files changed, 268 insertions(+), 162 deletions(-)

diff --git a/inventory/src/components/products/ProductFilters.tsx b/inventory/src/components/products/ProductFilters.tsx
index 56a80a3..0e5e739 100644
--- a/inventory/src/components/products/ProductFilters.tsx
+++ b/inventory/src/components/products/ProductFilters.tsx
@@ -231,32 +231,40 @@ export function ProductFilters({
   const [selectedFilter, setSelectedFilter] = React.useState<FilterOption | null>(null);
   const [selectedOperator, setSelectedOperator] = React.useState<ComparisonOperator>('=');
   const [inputValue, setInputValue] = React.useState("");
-  const [inputValue2, setInputValue2] = React.useState(""); // For 'between' operator
+  const [inputValue2, setInputValue2] = React.useState("");
   const [searchValue, setSearchValue] = React.useState("");
 
+  // Reset states when popup closes
+  const handlePopoverClose = () => {
+    setShowCommand(false);
+    setSelectedFilter(null);
+    setSelectedOperator('=');
+    setInputValue("");
+    setInputValue2("");
+    setSearchValue("");
+  };
+
   // Handle keyboard shortcuts
   React.useEffect(() => {
     const handleKeyDown = (e: KeyboardEvent) => {
       // Command/Ctrl + K to toggle filter
       if ((e.metaKey || e.ctrlKey) && e.key === 'k') {
         e.preventDefault();
-        setShowCommand(prev => !prev);
-      }
-      // Escape to close or go back
-      if (e.key === 'Escape') {
-        if (selectedFilter) {
-          setSelectedFilter(null);
-          setInputValue("");
+        if (!showCommand) {
+          setShowCommand(true);
         } else {
-          setShowCommand(false);
-          setSearchValue("");
+          handlePopoverClose();
         }
       }
+      // Only handle Escape at the root level
+      if (e.key === 'Escape' && !selectedFilter) {
+        handlePopoverClose();
+      }
     };
 
     window.addEventListener('keydown', handleKeyDown);
     return () => window.removeEventListener('keydown', handleKeyDown);
-  }, [selectedFilter]);
+  }, [selectedFilter, showCommand]);
 
   // Update filter options with dynamic data
   const filterOptions = React.useMemo(() => {
@@ -311,33 +319,16 @@ export function ProductFilters({
     };
     
     onFilterChange(newFilters as Record<string, ActiveFilterValue>);
+    handlePopoverClose();
+  };
+
+  const handleBackToFilters = () => {
     setSelectedFilter(null);
     setSelectedOperator('=');
     setInputValue("");
     setInputValue2("");
-    setSearchValue("");
   };
 
-  const handleKeyDown = React.useCallback((e: React.KeyboardEvent) => {
-    if (e.key === 'Enter' && selectedFilter) {
-      if (selectedFilter.type === 'select') {
-        const option = selectedFilter.options?.find(opt => 
-          opt.label.toLowerCase().includes(inputValue.toLowerCase())
-        );
-        if (option) {
-          handleApplyFilter(option.value);
-        }
-      } else if (selectedFilter.type === 'number') {
-        const numValue = parseFloat(inputValue);
-        if (!isNaN(numValue)) {
-          handleApplyFilter(numValue);
-        }
-      } else if (selectedFilter.type === 'text' && inputValue.trim() !== '') {
-        handleApplyFilter(inputValue.trim());
-      }
-    }
-  }, [selectedFilter, inputValue]);
-
   const activeFiltersList = React.useMemo(() => {
     if (!activeFilters) return [];
     
@@ -377,6 +368,15 @@ export function ProductFilters({
 
   const renderNumberInput = () => (
     <div className="flex flex-col gap-4 items-start">
+      <div className="mb-4">
+        <Button
+          variant="ghost"
+          onClick={handleBackToFilters}
+          className="text-muted-foreground"
+        >
+          ← Back to filters
+        </Button>
+      </div>
       {renderOperatorSelect()}
       <div className="flex items-center gap-2">
         <Input
@@ -400,6 +400,9 @@ export function ProductFilters({
                   handleApplyFilter(val);
                 }
               }
+            } else if (e.key === 'Escape') {
+              e.stopPropagation();
+              handleBackToFilters();
             }
           }}
           className="w-[120px] [appearance:textfield] [&::-webkit-outer-spin-button]:appearance-none [&::-webkit-inner-spin-button]:appearance-none"
@@ -419,6 +422,9 @@ export function ProductFilters({
                   if (!isNaN(val1) && !isNaN(val2)) {
                     handleApplyFilter([val1, val2]);
                   }
+                } else if (e.key === 'Escape') {
+                  e.stopPropagation();
+                  handleBackToFilters();
                 }
               }}
               className="w-[120px] [appearance:textfield] [&::-webkit-outer-spin-button]:appearance-none [&::-webkit-inner-spin-button]:appearance-none"
@@ -448,18 +454,23 @@ export function ProductFilters({
   );
 
   const getFilterDisplayValue = (filter: ActiveFilter) => {
-    if (Array.isArray(filter.value)) {
-      return `between ${filter.value[0]} and ${filter.value[1]}`;
+    const filterValue = activeFilters[filter.id];
+    const filterOption = filterOptions.find(opt => opt.id === filter.id);
+    
+    // For between ranges
+    if (Array.isArray(filterValue)) {
+      return `${filter.label} between ${filterValue[0]} and ${filterValue[1]}`;
     }
     
-    const filterValue = activeFilters[filter.id];
-    const operator = typeof filterValue === 'object' && 'operator' in filterValue
-      ? filterValue.operator
-      : '=';
-    const value = typeof filterValue === 'object' && 'value' in filterValue
-      ? filterValue.value
-      : filterValue;
-      
+    // For direct selections (select type) or text search
+    if (filterOption?.type === 'select' || filterOption?.type === 'text' || typeof filterValue !== 'object') {
+      const value = typeof filterValue === 'object' ? filterValue.value : filterValue;
+      return `${filter.label}: ${value}`;
+    }
+    
+    // For numeric filters with operators
+    const operator = filterValue.operator;
+    const value = filterValue.value;
     const operatorDisplay = {
       '=': '=',
       '>': '>',
@@ -469,13 +480,23 @@ export function ProductFilters({
       'between': 'between'
     }[operator];
     
-    return `${operatorDisplay} ${value}`;
+    return `${filter.label} ${operatorDisplay} ${value}`;
   };
 
   return (
     <div className="space-y-4">
       <div className="flex flex-wrap items-center gap-2">
-        <Popover open={showCommand} onOpenChange={setShowCommand}>
+        <Popover 
+          open={showCommand} 
+          onOpenChange={(open) => {
+            if (!open) {
+              handlePopoverClose();
+            } else {
+              setShowCommand(true);
+            }
+          }}
+          modal={true}
+        >
           <PopoverTrigger asChild>
             <Button
               variant="outline"
@@ -489,14 +510,37 @@ export function ProductFilters({
               {showCommand ? "Cancel" : "Add Filter"}
             </Button>
           </PopoverTrigger>
-          <PopoverContent className="p-0 w-[520px]" align="start">
-            <Command className="rounded-none border-0" shouldFilter={false}>
+          <PopoverContent 
+            className="p-0 w-[520px]" 
+            align="start"
+            onKeyDown={(e) => {
+              if (e.key === 'Escape') {
+                e.preventDefault();
+                e.stopPropagation();
+                if (selectedFilter) {
+                  handleBackToFilters();
+                } else {
+                  handlePopoverClose();
+                }
+              }
+            }}
+          >
+            <Command 
+              className="rounded-none border-0" 
+              shouldFilter={false}
+            >
               {!selectedFilter ? (
                 <>
                   <CommandInput 
                     placeholder="Search and select filters..." 
                     value={searchValue}
                     onValueChange={setSearchValue}
+                    onKeyDown={(e) => {
+                      if (e.key === 'Escape') {
+                        e.preventDefault();
+                        handlePopoverClose();
+                      }
+                    }}
                   />
                   <CommandList>
                     <CommandEmpty>No filters found.</CommandEmpty>
@@ -540,16 +584,90 @@ export function ProductFilters({
                 </>
               ) : selectedFilter.type === 'number' ? (
                 <div className="p-4">
-                  <div className="mb-4">
-                    <Button
-                      variant="ghost"
-                      onClick={() => setSelectedFilter(null)}
-                      className="text-muted-foreground"
-                    >
-                      ← Back to filters
-                    </Button>
+                  <div className="flex flex-col gap-4 items-start">
+                    <div className="mb-4">
+                      <Button
+                        variant="ghost"
+                        onClick={handleBackToFilters}
+                        className="text-muted-foreground"
+                      >
+                        ← Back to filters
+                      </Button>
+                    </div>
+                    {renderOperatorSelect()}
+                    <div className="flex items-center gap-2">
+                      <Input
+                        type="number"
+                        placeholder={`Enter ${selectedFilter?.label.toLowerCase()}`}
+                        value={inputValue}
+                        onChange={(e) => setInputValue(e.target.value)}
+                        onKeyDown={(e) => {
+                          if (e.key === 'Enter') {
+                            if (selectedOperator === 'between') {
+                              if (inputValue2) {
+                                const val1 = parseFloat(inputValue);
+                                const val2 = parseFloat(inputValue2);
+                                if (!isNaN(val1) && !isNaN(val2)) {
+                                  handleApplyFilter([val1, val2]);
+                                }
+                              }
+                            } else {
+                              const val = parseFloat(inputValue);
+                              if (!isNaN(val)) {
+                                handleApplyFilter(val);
+                              }
+                            }
+                          } else if (e.key === 'Escape') {
+                            e.preventDefault();
+                            handleBackToFilters();
+                          }
+                        }}
+                        className="w-[120px] [appearance:textfield] [&::-webkit-outer-spin-button]:appearance-none [&::-webkit-inner-spin-button]:appearance-none"
+                      />
+                      {selectedOperator === 'between' && (
+                        <>
+                          <span>and</span>
+                          <Input
+                            type="number"
+                            placeholder={`Enter maximum`}
+                            value={inputValue2}
+                            onChange={(e) => setInputValue2(e.target.value)}
+                            onKeyDown={(e) => {
+                              if (e.key === 'Enter') {
+                                const val1 = parseFloat(inputValue);
+                                const val2 = parseFloat(inputValue2);
+                                if (!isNaN(val1) && !isNaN(val2)) {
+                                  handleApplyFilter([val1, val2]);
+                                }
+                              } else if (e.key === 'Escape') {
+                                e.preventDefault();
+                                handleBackToFilters();
+                              }
+                            }}
+                            className="w-[120px] [appearance:textfield] [&::-webkit-outer-spin-button]:appearance-none [&::-webkit-inner-spin-button]:appearance-none"
+                          />
+                        </>
+                      )}
+                      <Button
+                        onClick={() => {
+                          if (selectedOperator === 'between') {
+                            const val1 = parseFloat(inputValue);
+                            const val2 = parseFloat(inputValue2);
+                            if (!isNaN(val1) && !isNaN(val2)) {
+                              handleApplyFilter([val1, val2]);
+                            }
+                          } else {
+                            const val = parseFloat(inputValue);
+                            if (!isNaN(val)) {
+                              handleApplyFilter(val);
+                            }
+                          }
+                        }}
+                      >
+                        Apply
+                      </Button>
+                    </div>
                   </div>
-                  {renderNumberInput()}
                 </div>
               ) : selectedFilter.type === 'select' ? (
                 <>
@@ -560,9 +678,10 @@ export function ProductFilters({
                     onKeyDown={(e) => {
                       if (e.key === 'Backspace' && !inputValue) {
                         e.preventDefault();
-                        setSelectedFilter(null);
-                      } else {
-                        handleKeyDown(e);
+                        handleBackToFilters();
+                      } else if (e.key === 'Escape') {
+                        e.preventDefault();
+                        handleBackToFilters();
                       }
                     }}
                   />
@@ -570,10 +689,7 @@ export function ProductFilters({
                     <CommandEmpty>No options found.</CommandEmpty>
                     <CommandGroup>
                       <CommandItem
-                        onSelect={() => {
-                          setSelectedFilter(null);
-                          setInputValue("");
-                        }}
+                        onSelect={handleBackToFilters}
                         className="cursor-pointer text-muted-foreground"
                       >
                         ← Back to filters
@@ -587,10 +703,7 @@ export function ProductFilters({
                           <CommandItem
                             key={option.value}
                             value={option.value}
-                            onSelect={() => {
-                              handleApplyFilter(option.value);
-                              setShowCommand(false);
-                            }}
+                            onSelect={() => handleApplyFilter(option.value)}
                             className="cursor-pointer"
                           >
                             {option.label}
@@ -603,68 +716,35 @@ export function ProductFilters({
               ) : (
                 <>
                   <CommandInput 
-                    placeholder={`Enter ${selectedFilter.label.toLowerCase()}`}
+                    placeholder={`Enter ${selectedFilter.label.toLowerCase()}...`}
                     value={inputValue}
-                    onValueChange={(value) => {
-                      if (selectedFilter.type === 'number') {
-                        if (/^\d*\.?\d*$/.test(value)) {
-                          setInputValue(value);
-                        }
-                      } else {
-                        setInputValue(value);
-                      }
-                    }}
+                    onValueChange={setInputValue}
                     onKeyDown={(e) => {
-                      if (e.key === 'Backspace' && !inputValue) {
+                      if (e.key === 'Enter' && inputValue.trim()) {
+                        handleApplyFilter(inputValue.trim());
+                      } else if (e.key === 'Escape') {
                         e.preventDefault();
-                        setSelectedFilter(null);
-                      } else if (e.key === 'Enter') {
-                        if (selectedFilter.type === 'number') {
-                          const numValue = parseFloat(inputValue);
-                          if (!isNaN(numValue)) {
-                            handleApplyFilter(numValue);
-                            setShowCommand(false);
-                          }
-                        } else {
-                          if (inputValue.trim() !== '') {
-                            handleApplyFilter(inputValue.trim());
-                            setShowCommand(false);
-                          }
-                        }
+                        handleBackToFilters();
                       }
                     }}
                   />
                   <CommandList>
                     <CommandGroup>
                       <CommandItem
-                        onSelect={() => {
-                          setSelectedFilter(null);
-                          setInputValue("");
-                        }}
+                        onSelect={handleBackToFilters}
                         className="cursor-pointer text-muted-foreground"
                       >
                         ← Back to filters
                       </CommandItem>
                       <CommandSeparator />
-                      <CommandItem
-                        onSelect={() => {
-                          if (selectedFilter.type === 'number') {
-                            const numValue = parseFloat(inputValue);
-                            if (!isNaN(numValue)) {
-                              handleApplyFilter(numValue);
-                              setShowCommand(false);
-                            }
-                          } else {
-                            if (inputValue.trim() !== '') {
-                              handleApplyFilter(inputValue.trim());
-                              setShowCommand(false);
-                            }
-                          }
-                        }}
-                        className="cursor-pointer"
-                      >
-                        Apply filter: {inputValue}
-                      </CommandItem>
+                      {inputValue.trim() && (
+                        <CommandItem
+                          onSelect={() => handleApplyFilter(inputValue.trim())}
+                          className="cursor-pointer"
+                        >
+                          Apply filter: {inputValue}
+                        </CommandItem>
+                      )}
                     </CommandGroup>
                   </CommandList>
                 </>
@@ -678,9 +758,7 @@ export function ProductFilters({
             variant="secondary"
             className="flex items-center gap-1"
           >
-            <span>
-              {filter.label}: {getFilterDisplayValue(filter)}
-            </span>
+            <span>{getFilterDisplayValue(filter)}</span>
             <Button
               variant="ghost"
               size="sm"
@@ -688,7 +766,6 @@ export function ProductFilters({
               onClick={() => {
                 const newFilters = { ...activeFilters };
                 delete newFilters[filter.id];
-                delete newFilters[`${filter.id}_operator`];
                 onFilterChange(newFilters);
               }}
             >
diff --git a/inventory/src/components/ui/dialog.tsx b/inventory/src/components/ui/dialog.tsx
index 9dbeaa0..1647513 100644
--- a/inventory/src/components/ui/dialog.tsx
+++ b/inventory/src/components/ui/dialog.tsx
@@ -1,3 +1,5 @@
+"use client"
+
 import * as React from "react"
 import * as DialogPrimitive from "@radix-ui/react-dialog"
 import { X } from "lucide-react"
diff --git a/inventory/src/pages/Products.tsx b/inventory/src/pages/Products.tsx
index 50322c6..1bba473 100644
--- a/inventory/src/pages/Products.tsx
+++ b/inventory/src/pages/Products.tsx
@@ -1,3 +1,4 @@
+import * as React from "react";
 import { useState, useEffect, useMemo } from 'react';
 import { useQuery, keepPreviousData } from '@tanstack/react-query';
 import { useSearchParams } from 'react-router-dom';
@@ -340,51 +341,77 @@ export function Products() {
     return acc;
   }, {} as Record<string, typeof AVAILABLE_COLUMNS>);
 
-  const renderColumnToggle = () => (
-    <DropdownMenu modal={false}>
-      <DropdownMenuTrigger asChild>
-        <Button variant="outline" className="ml-auto">
-          <Settings2 className="mr-2 h-4 w-4" />
-          Columns
-        </Button>
-      </DropdownMenuTrigger>
-      <DropdownMenuContent
-        align="end"
-        className="w-[500px] max-h-[calc(100vh-4rem)] overflow-y-auto"
-        onCloseAutoFocus={(e) => e.preventDefault()}
-      >
-        <DropdownMenuLabel className="sticky top-0 bg-background z-10">Toggle columns</DropdownMenuLabel>
-        <DropdownMenuSeparator className="sticky top-[29px] bg-background z-10" />
-        <div className="grid grid-cols-2 gap-4">
-          {Object.entries(columnsByGroup).map(([group, columns]) => (
-            <div key={group}>
-              <DropdownMenuLabel className="text-xs font-normal text-muted-foreground">
-                {group}
-              </DropdownMenuLabel>
-              {columns.map((column) => (
-                <DropdownMenuCheckboxItem
-                  key={column.key}
-                  className="capitalize"
-                  checked={visibleColumns.has(column.key)}
-                  onCheckedChange={(checked) => handleColumnVisibilityChange(column.key, checked)}
-                >
-                  {column.label}
-                </DropdownMenuCheckboxItem>
-              ))}
-            </div>
-          ))}
-        </div>
-        <DropdownMenuSeparator />
-        <Button 
-          variant="ghost" 
-          className="w-full justify-start" 
-          onClick={resetColumnsToDefault}
+  const renderColumnToggle = () => {
+    const [open, setOpen] = React.useState(false);
+    
+    return (
+      <DropdownMenu open={open} onOpenChange={setOpen} modal={false}>
+        <DropdownMenuTrigger asChild>
+          <Button variant="outline" className="ml-auto">
+            <Settings2 className="mr-2 h-4 w-4" />
+            Columns
+          </Button>
+        </DropdownMenuTrigger>
+        <DropdownMenuContent
+          align="end"
+          className="w-[500px] max-h-[calc(100vh-4rem)] overflow-y-auto"
+          onCloseAutoFocus={(e) => e.preventDefault()}
+          onPointerDownOutside={(e) => {
+            // Only close if clicking outside the dropdown
+            if (!(e.target as HTMLElement).closest('[role="dialog"]')) {
+              setOpen(false);
+            }
+          }}
+          onInteractOutside={(e) => {
+            // Prevent closing when interacting with checkboxes
+            if ((e.target as HTMLElement).closest('[role="dialog"]')) {
+              e.preventDefault();
+            }
+          }}
         >
-          Reset to Default
-        </Button>
-      </DropdownMenuContent>
-    </DropdownMenu>
-  );
+          <DropdownMenuLabel className="sticky top-0 bg-background z-10">Toggle columns</DropdownMenuLabel>
+          <DropdownMenuSeparator className="sticky top-[29px] bg-background z-10" />
+          <div className="grid grid-cols-2 gap-4">
+            {Object.entries(columnsByGroup).map(([group, columns]) => (
+              <div key={group}>
+                <DropdownMenuLabel className="text-xs font-normal text-muted-foreground">
+                  {group}
+                </DropdownMenuLabel>
+                {columns.map((column) => (
+                  <DropdownMenuCheckboxItem
+                    key={column.key}
+                    className="capitalize"
+                    checked={visibleColumns.has(column.key)}
+                    onCheckedChange={(checked) => {
+                      handleColumnVisibilityChange(column.key, checked);
+                    }}
+                    onSelect={(e) => {
+                      // Prevent closing by stopping propagation
+                      e.preventDefault();
+                    }}
+                  >
+                    {column.label}
+                  </DropdownMenuCheckboxItem>
+                ))}
+              </div>
+            ))}
+          </div>
+          <DropdownMenuSeparator />
+          <Button 
+            variant="ghost" 
+            className="w-full justify-start" 
+            onClick={(e) => {
+              resetColumnsToDefault();
+              // Prevent closing by stopping propagation
+              e.stopPropagation();
+            }}
+          >
+            Reset to Default
+          </Button>
+        </DropdownMenuContent>
+      </DropdownMenu>
+    );
+  };
 
   // Calculate pagination numbers
   const totalPages = data?.pagination.pages || 1;