Fix dropdown scrolling and keep multi-selects open

inventory/docs/fix-multi-select.md

@@ -0,0 +1,72 @@
+# Solution: Keeping Dropdowns Open During Multiple Selections
+
+## The Problem
+
+When implementing a multi-select dropdown in React, a common issue occurs:
+
+1. You select an item in the dropdown
+2. The `onChange` handler is called, updating the data
+3. This triggers a re-render of the parent component (in this case, the entire table)
+4. During the re-render, the dropdown is unmounted and remounted
+5. This causes the dropdown to close before you can make multiple selections
+
+## The Solution: Deferred State Updates
+
+The key insight is to **separate local state management from parent state updates**:
+
+```typescript
+// Step 1: Add local state to track selections 
+const [internalValue, setInternalValue] = useState<string[]>(value)
+
+// Step 2: Handle popover open state changes
+const handleOpenChange = useCallback((newOpen: boolean) => {
+  if (open && !newOpen) {
+    // Only update parent state when dropdown closes
+    if (JSON.stringify(internalValue) !== JSON.stringify(value)) {
+      onChange(internalValue);
+    }
+  }
+  
+  setOpen(newOpen);
+  
+  if (newOpen) {
+    // Sync internal state with external state when opening
+    setInternalValue(value);
+  }
+}, [open, internalValue, value, onChange]);
+
+// Step 3: Toggle selection only updates internal state
+const toggleSelection = useCallback((selectedValue: string) => {
+  setInternalValue(prev => {
+    if (prev.includes(selectedValue)) {
+      return prev.filter(v => v !== selectedValue);
+    } else {
+      return [...prev, selectedValue];
+    }
+  });
+}, []);
+```
+
+## Why This Works
+
+1. **No parent re-renders during selection**: Since we're only updating local state, the parent component doesn't re-render during selection.
+2. **Consistent UI**: The dropdown shows accurate selected states using the internal value.
+3. **Data integrity**: The final selections are properly synchronized back to the parent when done.
+4. **Resilient to external changes**: Initial state is synchronized when opening the dropdown.
+
+## Implementation Steps
+
+1. Create a local state variable to track selections inside the component
+2. Only make selections against this local state while the dropdown is open
+3. Defer updating the parent until the dropdown is explicitly closed
+4. When opening, synchronize the internal state with the external value
+
+## Benefits
+
+This pattern:
+- Avoids re-render cycles that would unmount the dropdown
+- Maintains UI consistency during multi-selection
+- Simplifies the component's interaction with parent components
+- Works with existing component lifecycles rather than fighting against them
+
+This solution is much simpler than trying to prevent event propagation or manipulating DOM events, and addresses the root cause of the issue: premature re-rendering.

inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/MultiInputCell.tsx

@@ -1,11 +1,11 @@
-import React, { useState, useCallback, useMemo } from 'react'
+import React, { useState, useCallback, useMemo, useEffect, useRef, useLayoutEffect } from 'react'
 import { Field } from '../../../../types'
 import { Textarea } from '@/components/ui/textarea'
 import { cn } from '@/lib/utils'
 import { Command, CommandEmpty, CommandGroup, CommandInput, CommandItem, CommandList } from '@/components/ui/command'
 import { Popover, PopoverContent, PopoverTrigger } from '@/components/ui/popover'
 import { Button } from '@/components/ui/button'
-import { Check, ChevronsUpDown, X } from 'lucide-react'
+import { Check, ChevronsUpDown } from 'lucide-react'
 import { Badge } from '@/components/ui/badge'
 
 // Define a type for field options
@@ -28,6 +28,9 @@ interface MultiInputCellProps<T extends string> {
   options?: readonly FieldOption[]
 }
 
+// Add global CSS to ensure fixed width constraints - use !important to override other styles
+const fixedWidthClass = "!w-full !min-w-0 !max-w-full !flex-shrink-1 !flex-grow-0";
+
 const MultiInputCell = <T extends string>({
   field,
   value = [],
@@ -42,6 +45,36 @@ const MultiInputCell = <T extends string>({
 }: MultiInputCellProps<T>) => {
   const [open, setOpen] = useState(false)
   const [searchQuery, setSearchQuery] = useState("")
+  // Add internal state for tracking selections
+  const [internalValue, setInternalValue] = useState<string[]>(value)
+  // Ref for the command list to enable scrolling
+  const commandListRef = useRef<HTMLDivElement>(null)
+
+  // Sync internalValue with external value when component mounts or value changes externally
+  useEffect(() => {
+    if (!open) {
+      setInternalValue(value)
+    }
+  }, [value, open])
+
+  // Handle open state changes with improved responsiveness
+  const handleOpenChange = useCallback((newOpen: boolean) => {
+    setOpen(newOpen);
+    
+    if (open && !newOpen) {
+      // Only update parent state when dropdown closes
+      // Avoid expensive deep comparison if lengths are different
+      if (internalValue.length !== value.length || 
+          internalValue.some((v, i) => v !== value[i])) {
+        onChange(internalValue);
+      }
+      if (onEndEdit) onEndEdit();
+    } else if (newOpen) {
+      // Sync internal state with external state when opening
+      setInternalValue(value);
+      if (onStartEdit) onStartEdit();
+    }
+  }, [open, internalValue, value, onChange, onStartEdit, onEndEdit]);
 
   // Memoize field options to prevent unnecessary recalculations
   const selectOptions = useMemo(() => {
@@ -74,28 +107,40 @@ const MultiInputCell = <T extends string>({
     );
   }, [selectOptions, searchQuery]);
 
+  // Sort options with selected items at the top for the dropdown
+  const sortedOptions = useMemo(() => {
+    return [...filteredOptions].sort((a, b) => {
+      const aSelected = internalValue.includes(a.value);
+      const bSelected = internalValue.includes(b.value);
+      
+      if (aSelected && !bSelected) return -1;
+      if (!aSelected && bSelected) return 1;
+      return a.label.localeCompare(b.label);
+    });
+  }, [filteredOptions, internalValue]);
+
   // Memoize selected values display
   const selectedValues = useMemo(() => {
-    return value.map(v => {
+    return internalValue.map(v => {
       const option = selectOptions.find(opt => String(opt.value) === String(v));
       return {
         value: v,
         label: option ? option.label : String(v)
       };
     });
-  }, [value, selectOptions]);
+  }, [internalValue, selectOptions]);
 
+  // Update the handleSelect to operate on internalValue instead of directly calling onChange
   const handleSelect = useCallback((selectedValue: string) => {
-    const newValue = value.includes(selectedValue)
+    setInternalValue(prev => {
-      ? value.filter(v => v !== selectedValue)
+      if (prev.includes(selectedValue)) {
-      : [...value, selectedValue];
+        return prev.filter(v => v !== selectedValue);
-    onChange(newValue);
+      } else {
+        return [...prev, selectedValue];
+      }
+    });
     setSearchQuery("");
-  }, [value, onChange]);
+  }, []);
-
-  const handleRemove = useCallback((valueToRemove: string) => {
-    onChange(value.filter(v => v !== valueToRemove));
-  }, [value, onChange]);
 
   // Handle focus
   const handleFocus = useCallback(() => {
@@ -109,8 +154,9 @@ const MultiInputCell = <T extends string>({
   
   // Handle direct input changes
   const handleInputChange = useCallback((e: React.ChangeEvent<HTMLInputElement | HTMLTextAreaElement>) => {
-    const newValue = e.target.value;
+    const newValue = e.target.value.split(separator).map(v => v.trim()).filter(Boolean);
-    onChange(newValue.split(separator).map(v => v.trim()).filter(Boolean));
+    setInternalValue(newValue);
+    onChange(newValue);
   }, [separator, onChange]);
   
   // Format display values for price
@@ -122,78 +168,162 @@ const MultiInputCell = <T extends string>({
       return !isNaN(numValue) ? `$${numValue.toFixed(2)}` : v.value
     });
   }, [selectedValues, isPrice]);
+
+  // Handle wheel scroll in dropdown
+  const handleWheel = useCallback((e: React.WheelEvent) => {
+    if (commandListRef.current) {
+      e.stopPropagation();
+      commandListRef.current.scrollTop += e.deltaY;
+    }
+  }, []);
   
   // Add outline even when not in focus
   const outlineClass = "border focus-visible:ring-0 focus-visible:ring-offset-0"
   
   // If we have a multi-select field with options, use command UI
   if (field.fieldType.type === 'multi-select' && selectOptions.length > 0) {
+    // Get width from field if available, or default to a reasonable value
+    const cellWidth = field.width || 200;
+    
+    // Create a reference to the container element
+    const containerRef = useRef<HTMLDivElement>(null);
+    
+    // Use a layout effect to force the width after rendering
+    useLayoutEffect(() => {
+      if (containerRef.current) {
+        const container = containerRef.current;
+        
+        // Force direct style properties using the DOM API
+        container.style.setProperty('width', `${cellWidth}px`, 'important');
+        container.style.setProperty('min-width', `${cellWidth}px`, 'important');
+        container.style.setProperty('max-width', `${cellWidth}px`, 'important');
+        container.style.setProperty('box-sizing', 'border-box', 'important');
+        container.style.setProperty('display', 'inline-block', 'important');
+        container.style.setProperty('flex', '0 0 auto', 'important');
+        
+        // Apply to the button element as well
+        const button = container.querySelector('button');
+        if (button) {
+          // Cast to HTMLElement to access style property
+          const htmlButton = button as HTMLElement;
+          htmlButton.style.setProperty('width', `${cellWidth}px`, 'important');
+          htmlButton.style.setProperty('min-width', `${cellWidth}px`, 'important');
+          htmlButton.style.setProperty('max-width', `${cellWidth}px`, 'important');
+          
+          // Make sure flex layout is enforced
+          const buttonContent = button.querySelector('div');
+          if (buttonContent && buttonContent instanceof HTMLElement) {
+            buttonContent.style.setProperty('display', 'flex', 'important');
+            buttonContent.style.setProperty('align-items', 'center', 'important');
+            buttonContent.style.setProperty('justify-content', 'space-between', 'important');
+            buttonContent.style.setProperty('width', '100%', 'important');
+            buttonContent.style.setProperty('overflow', 'hidden', 'important');
+          }
+          
+          // Find the chevron icon and ensure it's not wrapping
+          const chevron = button.querySelector('svg');
+          if (chevron && chevron instanceof SVGElement) {
+            chevron.style.cssText = 'flex-shrink: 0 !important; margin-left: auto !important;';
+          }
+        }
+      }
+    }, [cellWidth]);
+    
+    // Create a key-value map for inline styles with fixed width
+    const fixedWidth = {
+      width: `${cellWidth}px`,
+      minWidth: `${cellWidth}px`,
+      maxWidth: `${cellWidth}px`,
+      boxSizing: 'border-box' as const,
+      display: 'inline-block',
+      flex: '0 0 auto'
+    };
+    
     return (
-      <div className="flex flex-col gap-2">
+      <div 
-        <div className="flex flex-wrap gap-1">
+        ref={containerRef}
-          {selectedValues.map(({ value: val, label }) => (
+        className="inline-block fixed-width-cell overflow-visible"
-            <Badge
+        style={fixedWidth}
-              key={val}
+        data-width={cellWidth}
-              variant="secondary"
+      >
-              className={cn(
+        <Popover open={open} onOpenChange={handleOpenChange}>
-                "mr-1 mb-1",
-                hasErrors && "bg-red-100 hover:bg-red-200"
-              )}
-            >
-              {label}
-              <button
-                type="button"
-                className="ml-1 ring-offset-background rounded-full outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2"
-                onClick={() => handleRemove(val)}
-              >
-                <X className="h-3 w-3" />
-                <span className="sr-only">Remove</span>
-              </button>
-            </Badge>
-          ))}
-        </div>
-        <Popover open={open} onOpenChange={setOpen}>
           <PopoverTrigger asChild>
             <Button
               variant="outline"
               role="combobox"
               aria-expanded={open}
               className={cn(
-                "w-full justify-between font-normal",
+                "justify-between font-normal",
-                !value.length && "text-muted-foreground",
+                !internalValue.length && "text-muted-foreground",
-                hasErrors && "border-red-500"
+                hasErrors && "border-red-500",
+                "h-auto min-h-9 py-1"
               )}
-              onClick={handleFocus}
+              onClick={() => setOpen(true)}
+              style={fixedWidth}
             >
-              {value.length === 0 ? "Select..." : `${value.length} selected`}
+              <div className="flex items-center w-full justify-between">
-              <ChevronsUpDown className="ml-2 h-4 w-4 shrink-0 opacity-50" />
+                <div 
+                  className="flex items-center gap-2 overflow-hidden"
+                  style={{
+                    maxWidth: `${cellWidth - 32}px`,
+                  }}
+                >
+                  {internalValue.length === 0 ? (
+                    <span className="text-muted-foreground truncate w-full">Select...</span>
+                  ) : internalValue.length === 1 ? (
+                    <span className="truncate w-full">{selectedValues[0].label}</span>
+                  ) : (
+                    <>
+                      <Badge variant="secondary" className="shrink-0 whitespace-nowrap">
+                        {internalValue.length} selected
+                      </Badge>
+                      <span className="truncate" style={{ maxWidth: `${cellWidth - 100}px` }}>
+                        {selectedValues.map(v => v.label).join(', ')}
+                      </span>
+                    </>
+                  )}
+                </div>
+                <div className="ml-1 flex-none" style={{ width: '20px' }}>
+                  <ChevronsUpDown className="h-4 w-4 shrink-0 opacity-50" />
+                </div>
+              </div>
             </Button>
           </PopoverTrigger>
-          <PopoverContent className="w-[var(--radix-popover-trigger-width)] p-0" align="start">
+          <PopoverContent 
-            <Command shouldFilter={false}>
+            className="p-0" 
+            style={fixedWidth}
+            align="start" 
+            sideOffset={4}
+          >
+            <Command shouldFilter={false} className="overflow-hidden">
               <CommandInput
                 placeholder="Search options..."
                 className="h-9"
                 value={searchQuery}
                 onValueChange={setSearchQuery}
               />
-              <CommandList className="max-h-[200px] overflow-y-auto">
+              <CommandList 
+                className="max-h-[200px] overflow-y-auto" 
+                ref={commandListRef}
+                onWheel={handleWheel}
+              >
                 <CommandEmpty>No options found.</CommandEmpty>
                 <CommandGroup>
-                  {filteredOptions.map((option) => (
+                  {sortedOptions.map((option) => (
                     <CommandItem
                       key={option.value}
                       value={option.value}
                       onSelect={handleSelect}
+                      className="flex w-full"
                     >
-                      <div className="flex items-center">
+                      <div className="flex items-center w-full overflow-hidden">
                         <Check
                           className={cn(
-                            "mr-2 h-4 w-4",
+                            "mr-2 h-4 w-4 flex-shrink-0",
-                            value.includes(option.value) ? "opacity-100" : "opacity-0"
+                            internalValue.includes(option.value) ? "opacity-100" : "opacity-0"
                           )}
                         />
-                        {option.label}
+                        <span className="truncate w-full">{option.label}</span>
                       </div>
                     </CommandItem>
                   ))}
@@ -207,11 +337,98 @@ const MultiInputCell = <T extends string>({
   }
   
   // For standard multi-input without options, use text input
+  // Ensure the non-dropdown version also respects the width constraints
+  if (field.width) {
+    const cellWidth = field.width;
+    
+    // Create a reference to the container element
+    const containerRef = useRef<HTMLDivElement>(null);
+    
+    // Use a layout effect to force the width after rendering
+    useLayoutEffect(() => {
+      if (containerRef.current) {
+        const container = containerRef.current;
+        
+        // Force direct style properties using the DOM API
+        container.style.setProperty('width', `${cellWidth}px`, 'important');
+        container.style.setProperty('min-width', `${cellWidth}px`, 'important');
+        container.style.setProperty('max-width', `${cellWidth}px`, 'important');
+        container.style.setProperty('box-sizing', 'border-box', 'important');
+        container.style.setProperty('display', 'inline-block', 'important');
+        container.style.setProperty('flex', '0 0 auto', 'important');
+        
+        // Apply to the input or div element as well
+        const input = container.querySelector('textarea, div');
+        if (input) {
+          // Cast to HTMLElement to access style property
+          const htmlElement = input as HTMLElement;
+          htmlElement.style.setProperty('width', `${cellWidth}px`, 'important');
+          htmlElement.style.setProperty('min-width', `${cellWidth}px`, 'important');
+          htmlElement.style.setProperty('max-width', `${cellWidth}px`, 'important');
+        }
+      }
+    }, [cellWidth]);
+    
+    // Create a key-value map for inline styles with fixed width
+    const fixedWidth = {
+      width: `${cellWidth}px`,
+      minWidth: `${cellWidth}px`,
+      maxWidth: `${cellWidth}px`,
+      boxSizing: 'border-box' as const,
+      display: 'inline-block',
+      flex: '0 0 auto'
+    };
+    
+    return (
+      <div 
+        ref={containerRef}
+        className="inline-block fixed-width-cell"
+        style={fixedWidth}
+        data-width={cellWidth}
+      >
+        {isMultiline ? (
+          <Textarea
+            value={internalValue.join(separator)}
+            onChange={handleInputChange}
+            onFocus={handleFocus}
+            onBlur={handleBlur}
+            placeholder={`Enter values separated by ${separator}`}
+            className={cn(
+              "min-h-[80px] resize-none",
+              outlineClass,
+              hasErrors ? "border-destructive" : ""
+            )}
+            style={fixedWidth}
+          />
+        ) : (
+          <div
+            className={cn(
+              "flex h-9 rounded-md border px-3 py-1 text-sm",
+              "cursor-text truncate",
+              outlineClass,
+              hasErrors ? "border-destructive" : "",
+              "overflow-hidden items-center"
+            )}
+            onClick={handleFocus}
+            style={fixedWidth}
+          >
+            {internalValue.length > 0 ? getDisplayValues().join(`, `) : (
+              <span className="text-muted-foreground truncate">
+                {`Enter values separated by ${separator}`}
+              </span>
+            )}
+          </div>
+        )}
+      </div>
+    );
+  }
+  
+  // Fallback to default behavior if no width is specified
   return (
     <div className="w-full">
       {isMultiline ? (
         <Textarea
-          value={value.join(separator)}
+          value={internalValue.join(separator)}
           onChange={handleInputChange}
           onFocus={handleFocus}
           onBlur={handleBlur}
@@ -233,7 +450,7 @@ const MultiInputCell = <T extends string>({
           )}
           onClick={handleFocus}
         >
-          {value.length > 0 ? getDisplayValues().join(`, `) : (
+          {internalValue.length > 0 ? getDisplayValues().join(`, `) : (
             <span className="text-muted-foreground">
               {`Enter values separated by ${separator}`}
             </span>

inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/SelectCell.tsx

@@ -1,4 +1,4 @@
-import { useState } from 'react'
+import { useState, useRef, useCallback } from 'react'
 import { Field } from '../../../../types'
 import { Check, ChevronsUpDown } from 'lucide-react'
 import { Button } from '@/components/ui/button'
@@ -42,6 +42,8 @@ const SelectCell = <T extends string>({
   options
 }: SelectCellProps<T>) => {
   const [open, setOpen] = useState(false)
+  // Ref for the command list to enable scrolling
+  const commandListRef = useRef<HTMLDivElement>(null)
   
   // Ensure we always have an array of options with the correct shape
   const fieldType = field.fieldType;
@@ -66,6 +68,14 @@ const SelectCell = <T extends string>({
   const displayValue = value ? 
     selectOptions.find(option => String(option.value) === String(value))?.label || String(value) : 
     'Select...';
+
+  // Handle wheel scroll in dropdown
+  const handleWheel = useCallback((e: React.WheelEvent) => {
+    if (commandListRef.current) {
+      e.stopPropagation();
+      commandListRef.current.scrollTop += e.deltaY;
+    }
+  }, []);
   
   const handleSelect = (selectedValue: string) => {
     onChange(selectedValue);
@@ -98,7 +108,10 @@ const SelectCell = <T extends string>({
       <PopoverContent className="p-0" align="start">
         <Command>
           <CommandInput placeholder="Search..." />
-          <CommandList>
+          <CommandList 
+            ref={commandListRef} 
+            onWheel={handleWheel}
+          >
             <CommandEmpty>No results found.</CommandEmpty>
             <CommandGroup>
               {selectOptions.map((option) => (

inventory/src/pages/Import.tsx

@@ -325,7 +325,7 @@ const BASE_IMPORT_FIELDS = [
     key: "themes",
     description: "Product themes/styles",
     fieldType: {
-      type: "select",
+      type: "multi-select",
       options: [], // Will be populated from API
     },
     width: 300,
@@ -335,10 +335,10 @@ const BASE_IMPORT_FIELDS = [
     key: "colors",
     description: "Product colors",
     fieldType: {
-      type: "select",
+      type: "multi-select",
       options: [], // Will be populated from API
     },
-    width: 180,
+    width: 200,
   },
 ] as const;
 
@@ -444,7 +444,7 @@ export function Import() {
         return {
           ...field,
           fieldType: {
-            type: "select" as const,
+            type: "multi-select" as const,
             options: fieldOptions.colors || [],
           },
         };
@@ -492,7 +492,7 @@ export function Import() {
         return {
           ...field,
           fieldType: {
-            type: "select" as const,
+            type: "multi-select" as const,
             options: fieldOptions.themes || [],
           },
         };