diff --git a/inventory/docs/fix-multi-select.md b/inventory/docs/fix-multi-select.md new file mode 100644 index 0000000..38159c9 --- /dev/null +++ b/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(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. \ No newline at end of file diff --git a/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/MultiInputCell.tsx b/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/MultiInputCell.tsx index e7f3123..daa1333 100644 --- a/inventory/src/lib/react-spreadsheet-import/src/steps/ValidationStepNew/components/cells/MultiInputCell.tsx +++ b/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 { 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 = ({ field, value = [], @@ -42,6 +45,36 @@ const MultiInputCell = ({ }: MultiInputCellProps) => { const [open, setOpen] = useState(false) const [searchQuery, setSearchQuery] = useState("") + // Add internal state for tracking selections + const [internalValue, setInternalValue] = useState(value) + // Ref for the command list to enable scrolling + const commandListRef = useRef(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 = ({ ); }, [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) - ? value.filter(v => v !== selectedValue) - : [...value, selectedValue]; - onChange(newValue); + setInternalValue(prev => { + if (prev.includes(selectedValue)) { + return prev.filter(v => v !== selectedValue); + } 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 = ({ // Handle direct input changes const handleInputChange = useCallback((e: React.ChangeEvent) => { - const newValue = e.target.value; - onChange(newValue.split(separator).map(v => v.trim()).filter(Boolean)); + const newValue = e.target.value.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 = ({ 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(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 ( -
-
- {selectedValues.map(({ value: val, label }) => ( - - {label} - - - ))} -
- +
+ - - + + - + No options found. - {filteredOptions.map((option) => ( + {sortedOptions.map((option) => ( -
+
- {option.label} + {option.label}
))} @@ -207,11 +337,98 @@ const MultiInputCell = ({ } // 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(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 ( +
+ {isMultiline ? ( +