From 5e2ee73e2d8006f8b511ed9483c9d0f052e495f1 Mon Sep 17 00:00:00 2001 From: Matt Date: Sat, 6 Sep 2025 15:08:53 -0400 Subject: [PATCH] Product import speed/responsiveness fixes, particularly around validation --- .../components/UpcValidationTableAdapter.tsx | 12 +- .../components/ValidationCell.tsx | 41 ++++- .../components/ValidationContainer.tsx | 6 +- .../components/ValidationTable.tsx | 17 +- .../components/cells/SelectCell.tsx | 4 +- .../hooks/useRowOperations.tsx | 2 +- .../hooks/useUniqueItemNumbersValidation.tsx | 69 ++++---- .../hooks/useValidationState.tsx | 162 ++++++++++-------- 8 files changed, 199 insertions(+), 114 deletions(-) diff --git a/inventory/src/components/product-import/steps/ValidationStepNew/components/UpcValidationTableAdapter.tsx b/inventory/src/components/product-import/steps/ValidationStepNew/components/UpcValidationTableAdapter.tsx index 80980c4..e805244 100644 --- a/inventory/src/components/product-import/steps/ValidationStepNew/components/UpcValidationTableAdapter.tsx +++ b/inventory/src/components/product-import/steps/ValidationStepNew/components/UpcValidationTableAdapter.tsx @@ -20,6 +20,8 @@ interface UpcValidationTableAdapterProps { copyDown: (rowIndex: number, fieldKey: string, endRowIndex?: number) => void validatingCells: Set isLoadingTemplates: boolean + editingCells: Set + setEditingCells: React.Dispatch>> rowProductLines: Record rowSublines: Record isLoadingLines: Record @@ -53,6 +55,8 @@ function UpcValidationTableAdapter({ copyDown, validatingCells: externalValidatingCells, isLoadingTemplates, + editingCells, + setEditingCells, rowProductLines, rowSublines, isLoadingLines, @@ -86,11 +90,7 @@ function UpcValidationTableAdapter({ // First add from itemNumbers directly - this is the source of truth for template applications if (itemNumbers) { - // Log all numbers for debugging - console.log(`[ADAPTER-DEBUG] Received itemNumbers map with ${itemNumbers.size} entries`); - itemNumbers.forEach((itemNumber, rowIndex) => { - console.log(`[ADAPTER-DEBUG] Adding item number for row ${rowIndex} from itemNumbers: ${itemNumber}`); result.set(rowIndex, itemNumber); }); } @@ -100,14 +100,12 @@ function UpcValidationTableAdapter({ // Check if upcValidation has an item number for this row const itemNumber = upcValidation.getItemNumber(index); if (itemNumber) { - console.log(`[ADAPTER-DEBUG] Adding item number for row ${index} from upcValidation: ${itemNumber}`); result.set(index, itemNumber); } // Also check if it's directly in the data const dataItemNumber = data[index].item_number; if (dataItemNumber && !result.has(index)) { - console.log(`[ADAPTER-DEBUG] Adding item number for row ${index} from data: ${dataItemNumber}`); result.set(index, dataItemNumber); } }); @@ -151,6 +149,8 @@ function UpcValidationTableAdapter({ rowSublines={rowSublines} isLoadingLines={isLoadingLines} isLoadingSublines={isLoadingSublines} + editingCells={editingCells} + setEditingCells={setEditingCells} /> ) } diff --git a/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationCell.tsx b/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationCell.tsx index 0c5fd91..9750bc3 100644 --- a/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationCell.tsx +++ b/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationCell.tsx @@ -78,7 +78,9 @@ const BaseCellContent = React.memo(({ hasErrors, options = [], className = '', - fieldKey = '' + fieldKey = '', + onStartEdit, + onEndEdit }: { field: Field; value: any; @@ -87,6 +89,8 @@ const BaseCellContent = React.memo(({ options?: readonly any[]; className?: string; fieldKey?: string; + onStartEdit?: () => void; + onEndEdit?: () => void; }) => { // Get field type information const fieldType = fieldKey === 'line' || fieldKey === 'subline' @@ -113,6 +117,8 @@ const BaseCellContent = React.memo(({ field={{...field, fieldType: { type: 'select', options }}} value={value} onChange={onChange} + onStartEdit={onStartEdit} + onEndEdit={onEndEdit} options={options} hasErrors={hasErrors} className={className} @@ -127,6 +133,8 @@ const BaseCellContent = React.memo(({ field={field} value={value} onChange={onChange} + onStartEdit={onStartEdit} + onEndEdit={onEndEdit} options={options} hasErrors={hasErrors} className={className} @@ -141,6 +149,8 @@ const BaseCellContent = React.memo(({ field={field} value={value} onChange={onChange} + onStartEdit={onStartEdit} + onEndEdit={onEndEdit} options={options} hasErrors={hasErrors} className={className} @@ -154,6 +164,8 @@ const BaseCellContent = React.memo(({ field={field} value={value} onChange={onChange} + onStartEdit={onStartEdit} + onEndEdit={onEndEdit} hasErrors={hasErrors} isMultiline={isMultiline} isPrice={isPrice} @@ -191,6 +203,8 @@ export interface ValidationCellProps { rowIndex: number copyDown?: (endRowIndex?: number) => void totalRows?: number + editingCells: Set + setEditingCells: React.Dispatch>> } // Add efficient error message extraction function @@ -288,7 +302,9 @@ const ValidationCell = React.memo(({ width, copyDown, rowIndex, - totalRows = 0 + totalRows = 0, + editingCells, + setEditingCells }: ValidationCellProps) => { // Use the CopyDown context const copyDownContext = React.useContext(CopyDownContext); @@ -297,9 +313,6 @@ const ValidationCell = React.memo(({ // This ensures that when the itemNumber changes, the display value changes let displayValue; if (fieldKey === 'item_number' && itemNumber) { - // Always log when an item_number field is rendered to help debug - console.log(`[VC-DEBUG] ValidationCell rendering item_number for row=${rowIndex} with itemNumber=${itemNumber}, value=${value}`); - // Prioritize itemNumber prop for item_number fields displayValue = itemNumber; } else { @@ -324,6 +337,22 @@ const ValidationCell = React.memo(({ // Add state for hover on target row const [isTargetRowHovered, setIsTargetRowHovered] = React.useState(false); + // PERFORMANCE FIX: Create cell key for editing state management + const cellKey = `${rowIndex}-${fieldKey}`; + + // SINGLE-CLICK EDITING FIX: Create editing state management functions + const handleStartEdit = React.useCallback(() => { + setEditingCells(prev => new Set([...prev, cellKey])); + }, [setEditingCells, cellKey]); + + const handleEndEdit = React.useCallback(() => { + setEditingCells(prev => { + const newSet = new Set(prev); + newSet.delete(cellKey); + return newSet; + }); + }, [setEditingCells, cellKey]); + // Force isValidating to be a boolean const isLoading = isValidating === true; @@ -461,6 +490,8 @@ const ValidationCell = React.memo(({ options={options} className={cellClassName} fieldKey={fieldKey} + onStartEdit={handleStartEdit} + onEndEdit={handleEndEdit} /> )} diff --git a/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationContainer.tsx b/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationContainer.tsx index 301094e..9c7adf3 100644 --- a/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationContainer.tsx +++ b/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationContainer.tsx @@ -61,7 +61,9 @@ const ValidationContainer = ({ fields, isLoadingTemplates, validatingCells, - setValidatingCells + setValidatingCells, + editingCells, + setEditingCells } = validationState // Use product lines fetching hook @@ -941,6 +943,8 @@ const ValidationContainer = ({ filters={filters} templates={templates} applyTemplate={applyTemplateWrapper} + editingCells={editingCells} + setEditingCells={setEditingCells} getTemplateDisplayText={getTemplateDisplayText} isValidatingUpc={isRowValidatingUpc} validatingUpcRows={Array.from(upcValidation.validatingRows)} diff --git a/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationTable.tsx b/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationTable.tsx index 8ff4050..ee251ca 100644 --- a/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationTable.tsx +++ b/inventory/src/components/product-import/steps/ValidationStepNew/components/ValidationTable.tsx @@ -46,6 +46,8 @@ interface ValidationTableProps { itemNumbers: Map isLoadingTemplates?: boolean copyDown: (rowIndex: number, key: string, endRowIndex?: number) => void + editingCells: Set + setEditingCells: React.Dispatch>> [key: string]: any } @@ -106,7 +108,9 @@ const MemoizedCell = React.memo(({ width, rowIndex, copyDown, - totalRows + totalRows, + editingCells, + setEditingCells }: { field: Field, value: any, @@ -119,7 +123,9 @@ const MemoizedCell = React.memo(({ width: number, rowIndex: number, copyDown?: (endRowIndex?: number) => void, - totalRows: number + totalRows: number, + editingCells: Set, + setEditingCells: React.Dispatch>> }) => { return ( ); }, (prev, next) => { @@ -146,6 +154,7 @@ const MemoizedCell = React.memo(({ } // Simplified memo comparison - most expensive checks removed + // Note: editingCells changes are not checked here as they need immediate re-renders return prev.value === next.value && prev.isValidating === next.isValidating && prev.errors === next.errors && @@ -169,6 +178,8 @@ const ValidationTable = ({ itemNumbers, isLoadingTemplates = false, copyDown, + editingCells, + setEditingCells, rowProductLines = {}, rowSublines = {}, isLoadingLines = {}, @@ -463,6 +474,8 @@ const ValidationTable = ({ rowIndex={row.index} copyDown={(endRowIndex?: number) => handleCopyDown(row.index, field.key as string, endRowIndex)} totalRows={data.length} + editingCells={editingCells} + setEditingCells={setEditingCells} /> ); } diff --git a/inventory/src/components/product-import/steps/ValidationStepNew/components/cells/SelectCell.tsx b/inventory/src/components/product-import/steps/ValidationStepNew/components/cells/SelectCell.tsx index 97e8fc7..6fcd875 100644 --- a/inventory/src/components/product-import/steps/ValidationStepNew/components/cells/SelectCell.tsx +++ b/inventory/src/components/product-import/steps/ValidationStepNew/components/cells/SelectCell.tsx @@ -142,10 +142,10 @@ const SelectCell = ({ // 5. Call onChange synchronously to avoid race conditions with other cells onChange(valueToCommit); - // 6. Clear processing state after a short delay + // 6. Clear processing state after a short delay - reduced for responsiveness setTimeout(() => { setIsProcessing(false); - }, 200); + }, 50); }, [onChange, onEndEdit]); // If disabled, render a static view diff --git a/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useRowOperations.tsx b/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useRowOperations.tsx index 3fa4363..7b22f57 100644 --- a/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useRowOperations.tsx +++ b/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useRowOperations.tsx @@ -255,7 +255,7 @@ export const useRowOperations = ( return newData; }); } - }, 50); + }, 5); // Reduced delay for faster secondary effects }, [data, fields, validateFieldFromHook, setData, setValidationErrors] ); diff --git a/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useUniqueItemNumbersValidation.tsx b/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useUniqueItemNumbersValidation.tsx index a1439c8..62bcf0f 100644 --- a/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useUniqueItemNumbersValidation.tsx +++ b/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useUniqueItemNumbersValidation.tsx @@ -10,8 +10,6 @@ export const useUniqueItemNumbersValidation = ( ) => { // Update validateUniqueItemNumbers to also check for uniqueness of UPC/barcode const validateUniqueItemNumbers = useCallback(async () => { - console.log("Validating unique fields"); - // Skip if no data if (!data.length) return; @@ -23,11 +21,6 @@ export const useUniqueItemNumbersValidation = ( .filter((field) => field.validations?.some((v) => v.rule === "unique")) .map((field) => String(field.key)); - console.log( - `Found ${uniqueFields.length} fields requiring uniqueness validation:`, - uniqueFields - ); - // Always check item_number uniqueness even if not explicitly defined if (!uniqueFields.includes("item_number")) { uniqueFields.push("item_number"); @@ -41,32 +34,44 @@ export const useUniqueItemNumbersValidation = ( // Initialize batch updates const errors = new Map>(); - // Single pass through data to identify all unique values - data.forEach((row, index) => { - uniqueFields.forEach((fieldKey) => { - const value = row[fieldKey as keyof typeof row]; + // ASYNC: Single pass through data to identify all unique values in batches + const BATCH_SIZE = 20; + for (let batchStart = 0; batchStart < data.length; batchStart += BATCH_SIZE) { + const batchEnd = Math.min(batchStart + BATCH_SIZE, data.length); + + for (let index = batchStart; index < batchEnd; index++) { + const row = data[index]; + uniqueFields.forEach((fieldKey) => { + const value = row[fieldKey as keyof typeof row]; - // Skip empty values - if (value === undefined || value === null || value === "") { - return; - } + // Skip empty values + if (value === undefined || value === null || value === "") { + return; + } - const valueStr = String(value); - const fieldMap = uniqueFieldsMap.get(fieldKey); + const valueStr = String(value); + const fieldMap = uniqueFieldsMap.get(fieldKey); - if (fieldMap) { - // Get or initialize the array of indices for this value - const indices = fieldMap.get(valueStr) || []; - indices.push(index); - fieldMap.set(valueStr, indices); - } - }); - }); + if (fieldMap) { + // Get or initialize the array of indices for this value + const indices = fieldMap.get(valueStr) || []; + indices.push(index); + fieldMap.set(valueStr, indices); + } + }); + } + + // Yield control back to UI thread after each batch + if (batchEnd < data.length) { + await new Promise(resolve => setTimeout(resolve, 0)); + } + } - // Process duplicates - uniqueFields.forEach((fieldKey) => { + // ASYNC: Process duplicates in batches to prevent UI blocking + let processedFields = 0; + for (const fieldKey of uniqueFields) { const fieldMap = uniqueFieldsMap.get(fieldKey); - if (!fieldMap) return; + if (!fieldMap) continue; fieldMap.forEach((indices, value) => { // Only process if there are duplicates @@ -93,7 +98,13 @@ export const useUniqueItemNumbersValidation = ( }); } }); - }); + + processedFields++; + // Yield control after every few fields to prevent UI blocking + if (processedFields % 2 === 0) { + await new Promise(resolve => setTimeout(resolve, 0)); + } + } // Merge uniqueness errors with existing validation errors setValidationErrors((prev) => { diff --git a/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useValidationState.tsx b/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useValidationState.tsx index 636c35c..1c9399d 100644 --- a/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useValidationState.tsx +++ b/inventory/src/components/product-import/steps/ValidationStepNew/hooks/useValidationState.tsx @@ -89,6 +89,10 @@ export const useValidationState = ({ // Add state for tracking cells in loading state const [validatingCells, setValidatingCells] = useState>(new Set()); + + // Add global editing state to prevent validation during editing + const [editingCells, setEditingCells] = useState>(new Set()); + const hasEditingCells = editingCells.size > 0; const initialValidationDoneRef = useRef(false); const isValidatingRef = useRef(false); @@ -138,16 +142,20 @@ export const useValidationState = ({ // CRITICAL FIX: Skip validation if we're already validating to prevent infinite loops if (isValidatingRef.current) return; + + // PERFORMANCE FIX: Skip validation while cells are being edited + if (hasEditingCells) return; - // Debounce validation to prevent excessive calls + // Debounce validation to prevent excessive calls - reduced for better responsiveness const timeoutId = setTimeout(() => { if (isValidatingRef.current) return; // Double-check before proceeding + if (hasEditingCells) return; // Double-check editing state // Validation running (removed console.log for performance) isValidatingRef.current = true; - // COMPREHENSIVE validation that clears old errors and adds new ones - const validateFields = () => { + // ASYNC validation that clears old errors and adds new ones + const validateFields = async () => { try { // Create a complete fresh validation map const allValidationErrors = new Map>(); @@ -160,89 +168,103 @@ export const useValidationState = ({ field.validations?.some((v) => v.rule === "regex") ); - // Validate each row completely - data.forEach((row, rowIndex) => { - const rowErrors: Record = {}; + // ASYNC PROCESSING: Process rows in small batches to prevent UI blocking + const BATCH_SIZE = 10; // Small batch size for responsiveness + const totalRows = data.length; + + for (let batchStart = 0; batchStart < totalRows; batchStart += BATCH_SIZE) { + const batchEnd = Math.min(batchStart + BATCH_SIZE, totalRows); + + // Process this batch synchronously (fast) + for (let rowIndex = batchStart; rowIndex < batchEnd; rowIndex++) { + const row = data[rowIndex]; + const rowErrors: Record = {}; - // Check required fields - requiredFields.forEach((field) => { - const key = String(field.key); - const value = row[key as keyof typeof row]; - - // Check if field is empty - if (value === undefined || value === null || value === "" || - (Array.isArray(value) && value.length === 0)) { - const requiredValidation = field.validations?.find((v) => v.rule === "required"); - rowErrors[key] = [ - { - message: requiredValidation?.errorMessage || "This field is required", - level: requiredValidation?.level || "error", - source: "row", - type: "required", - }, - ]; - } - }); - - // Check regex fields (only if they have values) - regexFields.forEach((field) => { - const key = String(field.key); - const value = row[key as keyof typeof row]; - - // Skip empty values for regex validation - if (value === undefined || value === null || value === "") { - return; - } - - const regexValidation = field.validations?.find((v) => v.rule === "regex"); - if (regexValidation) { - try { - const regex = new RegExp(regexValidation.value, regexValidation.flags); - if (!regex.test(String(value))) { - // Only add regex error if no required error exists - if (!rowErrors[key]) { - rowErrors[key] = [ - { - message: regexValidation.errorMessage, - level: regexValidation.level || "error", - source: "row", - type: "regex", - }, - ]; - } - } - } catch (error) { - console.error("Invalid regex in validation:", error); + // Check required fields + requiredFields.forEach((field) => { + const key = String(field.key); + const value = row[key as keyof typeof row]; + + // Check if field is empty + if (value === undefined || value === null || value === "" || + (Array.isArray(value) && value.length === 0)) { + const requiredValidation = field.validations?.find((v) => v.rule === "required"); + rowErrors[key] = [ + { + message: requiredValidation?.errorMessage || "This field is required", + level: requiredValidation?.level || "error", + source: "row", + type: "required", + }, + ]; } - } - }); + }); - // Only add to the map if there are actually errors - if (Object.keys(rowErrors).length > 0) { - allValidationErrors.set(rowIndex, rowErrors); + // Check regex fields (only if they have values) + regexFields.forEach((field) => { + const key = String(field.key); + const value = row[key as keyof typeof row]; + + // Skip empty values for regex validation + if (value === undefined || value === null || value === "") { + return; + } + + const regexValidation = field.validations?.find((v) => v.rule === "regex"); + if (regexValidation) { + try { + const regex = new RegExp(regexValidation.value, regexValidation.flags); + if (!regex.test(String(value))) { + // Only add regex error if no required error exists + if (!rowErrors[key]) { + rowErrors[key] = [ + { + message: regexValidation.errorMessage, + level: regexValidation.level || "error", + source: "row", + type: "regex", + }, + ]; + } + } + } catch (error) { + console.error("Invalid regex in validation:", error); + } + } + }); + + // Only add to the map if there are actually errors + if (Object.keys(rowErrors).length > 0) { + allValidationErrors.set(rowIndex, rowErrors); + } } - }); + + // CRITICAL: Yield control back to the UI thread after each batch + if (batchEnd < totalRows) { + await new Promise(resolve => setTimeout(resolve, 0)); + } + } // Replace validation errors completely (clears old ones) setValidationErrors(allValidationErrors); - // Run uniqueness validations after basic validation - validateUniqueItemNumbers(); + // Run uniqueness validations after basic validation (also async) + setTimeout(() => validateUniqueItemNumbers(), 0); } finally { - // Always ensure the ref is reset, even if an error occurs + // Always ensure the ref is reset, even if an error occurs - reduced delay setTimeout(() => { isValidatingRef.current = false; - }, 100); + }, 10); } }; - // Run validation immediately + // Run validation immediately (async) validateFields(); - }, 50); // 50ms debounce + }, 10); // Reduced debounce for better responsiveness // Cleanup timeout on unmount or dependency change return () => clearTimeout(timeoutId); - }, [data, fields]); // Removed validateUniqueItemNumbers to prevent infinite loop + }, [data, fields, hasEditingCells]); // Added hasEditingCells to dependencies // Add field options query const { data: fieldOptionsData } = useQuery({ @@ -689,6 +711,10 @@ export const useValidationState = ({ // CRITICAL: Export validatingCells to make it available to ValidationContainer validatingCells, setValidatingCells, + + // PERFORMANCE FIX: Export editing state management + editingCells, + setEditingCells, // Row selection rowSelection,