Restore line and subline fields
This commit is contained in:
239
docs/validate-table-changes-implementation-issue4.md
Normal file
239
docs/validate-table-changes-implementation-issue4.md
Normal file
@@ -0,0 +1,239 @@
|
||||
# Validation Display Issue Implementation
|
||||
|
||||
## Issue Being Addressed
|
||||
|
||||
**Validation Display Issue**: Validation isn't happening beyond checking if a cell is required or not. All validation rules defined in import.tsx need to be respected.
|
||||
* Required fields correctly show a red border when empty (✅ ALREADY WORKING)
|
||||
* Non-empty fields with validation errors (regex, unique, etc.) should show a red border AND an alert circle icon with tooltip explaining the error (❌ NOT WORKING)
|
||||
|
||||
## Implementation Attempts
|
||||
|
||||
!!!!**NOTE** All previous attempts have been reverted and are no longer part of the code, please take this into account when trying a new solution. !!!!
|
||||
|
||||
### Attempt 1: Fix Validation Display Logic
|
||||
|
||||
**Approach**: Modified `processErrors` function to separate required errors from validation errors and show alert icons only for non-empty fields with validation errors.
|
||||
|
||||
**Changes Made**:
|
||||
```typescript
|
||||
function processErrors(value: any, errors: ErrorObject[]) {
|
||||
// ...existing code...
|
||||
|
||||
// Separate required errors from other validation errors
|
||||
const requiredErrors = errors.filter(error =>
|
||||
error.message?.toLowerCase().includes('required')
|
||||
);
|
||||
const validationErrors = errors.filter(error =>
|
||||
!error.message?.toLowerCase().includes('required')
|
||||
);
|
||||
|
||||
const isRequiredButEmpty = valueIsEmpty && requiredErrors.length > 0;
|
||||
const hasValidationErrors = validationErrors.length > 0;
|
||||
const shouldShowErrorIcon = hasValidationErrors && !valueIsEmpty;
|
||||
|
||||
// ...more code...
|
||||
}
|
||||
```
|
||||
|
||||
**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip.
|
||||
|
||||
### Attempt 2: Comprehensive Fix for Validation Display
|
||||
|
||||
**Approach**: Completely rewrote `processErrors` function with consistent empty value detection, clear error separation, and improved error message extraction.
|
||||
|
||||
**Changes Made**:
|
||||
```typescript
|
||||
function processErrors(value: any, errors: ErrorObject[]) {
|
||||
if (!errors || errors.length === 0) {
|
||||
return { filteredErrors: [], hasError: false, isRequiredButEmpty: false,
|
||||
shouldShowErrorIcon: false, errorMessages: '' };
|
||||
}
|
||||
|
||||
const valueIsEmpty = isEmpty(value);
|
||||
const requiredErrors = errors.filter(error =>
|
||||
error.message?.toLowerCase().includes('required')
|
||||
);
|
||||
const validationErrors = errors.filter(error =>
|
||||
!error.message?.toLowerCase().includes('required')
|
||||
);
|
||||
|
||||
let filteredErrors = valueIsEmpty ? requiredErrors : validationErrors;
|
||||
|
||||
const isRequiredButEmpty = valueIsEmpty && requiredErrors.length > 0;
|
||||
const hasValidationErrors = validationErrors.length > 0;
|
||||
const hasError = isRequiredButEmpty || hasValidationErrors;
|
||||
const shouldShowErrorIcon = hasValidationErrors && !valueIsEmpty;
|
||||
|
||||
let errorMessages = '';
|
||||
if (shouldShowErrorIcon) {
|
||||
errorMessages = validationErrors.map(getErrorMessage).join('\n');
|
||||
}
|
||||
|
||||
return { filteredErrors, hasError, isRequiredButEmpty, shouldShowErrorIcon, errorMessages };
|
||||
}
|
||||
```
|
||||
|
||||
**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip.
|
||||
|
||||
### Attempt 3: Simplified Error Processing Logic
|
||||
|
||||
**Approach**: Refactored `processErrors` to use shared `isEmpty` function, simplified error icon logic, and made error message extraction more direct.
|
||||
|
||||
**Changes Made**:
|
||||
```typescript
|
||||
function processErrors(value: any, errors: ErrorObject[]) {
|
||||
if (!errors || errors.length === 0) {
|
||||
return { filteredErrors: [], hasError: false, isRequiredButEmpty: false,
|
||||
shouldShowErrorIcon: false, errorMessages: '' };
|
||||
}
|
||||
|
||||
const valueIsEmpty = isEmpty(value);
|
||||
const requiredErrors = errors.filter(error =>
|
||||
error.message?.toLowerCase().includes('required')
|
||||
);
|
||||
const validationErrors = errors.filter(error =>
|
||||
!error.message?.toLowerCase().includes('required')
|
||||
);
|
||||
|
||||
let filteredErrors = valueIsEmpty ? requiredErrors : validationErrors;
|
||||
|
||||
const isRequiredButEmpty = valueIsEmpty && requiredErrors.length > 0;
|
||||
const hasValidationErrors = !valueIsEmpty && validationErrors.length > 0;
|
||||
const hasError = isRequiredButEmpty || hasValidationErrors;
|
||||
const shouldShowErrorIcon = hasValidationErrors;
|
||||
|
||||
let errorMessages = '';
|
||||
if (shouldShowErrorIcon) {
|
||||
errorMessages = validationErrors.map(getErrorMessage).join('\n');
|
||||
}
|
||||
|
||||
return { filteredErrors, hasError, isRequiredButEmpty, shouldShowErrorIcon, errorMessages };
|
||||
}
|
||||
```
|
||||
|
||||
**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip.
|
||||
|
||||
### Attempt 4: Consistent Error Processing Across Components
|
||||
|
||||
**Approach**: Updated both `processErrors` function and `ValidationCell` component to ensure consistent error handling between components.
|
||||
|
||||
**Changes Made**:
|
||||
```typescript
|
||||
// In processErrors function
|
||||
function processErrors(value: any, errors: ErrorObject[]) {
|
||||
// Similar to Attempt 3 with consistent error handling
|
||||
}
|
||||
|
||||
// In ValidationCell component
|
||||
const ValidationCell = ({ field, value, onChange, errors, /* other props */ }) => {
|
||||
// ...existing code...
|
||||
|
||||
// Use the processErrors function to handle validation errors
|
||||
const { hasError, isRequiredButEmpty, shouldShowErrorIcon, errorMessages } =
|
||||
React.useMemo(() => processErrors(value, errors), [value, errors]);
|
||||
|
||||
// ...rest of the component...
|
||||
}
|
||||
```
|
||||
|
||||
**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip.
|
||||
|
||||
### Attempt 5: Unified Error Processing with ItemNumberCell
|
||||
|
||||
**Approach**: Replaced custom error processing in `ValidationCell` with the same `processErrors` utility used by `ItemNumberCell`.
|
||||
|
||||
**Changes Made**:
|
||||
```typescript
|
||||
const ValidationCell = ({ field, value, onChange, errors, /* other props */ }) => {
|
||||
// State and context setup...
|
||||
|
||||
// For item_number fields, use the specialized component
|
||||
if (fieldKey === 'item_number') {
|
||||
return <ItemNumberCell {...props} />;
|
||||
}
|
||||
|
||||
// Use the same processErrors utility function that ItemNumberCell uses
|
||||
const { hasError, isRequiredButEmpty, shouldShowErrorIcon, errorMessages } =
|
||||
React.useMemo(() => processErrors(value, errors), [value, errors]);
|
||||
|
||||
// Rest of component...
|
||||
}
|
||||
```
|
||||
|
||||
**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip.
|
||||
|
||||
### Attempt 6: Standardize Error Processing Across Cell Types
|
||||
|
||||
**Approach**: Standardized error handling across all cell types using the shared `processErrors` utility function.
|
||||
|
||||
**Changes Made**: Similar to Attempt 5, with focus on standardizing the approach for determining when to show validation error icons.
|
||||
|
||||
**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip.
|
||||
|
||||
### Attempt 7: Replace Custom Error Processing with Shared Utility
|
||||
|
||||
**Approach**: Ensured consistent error handling between `ItemNumberCell` and regular `ValidationCell` components.
|
||||
|
||||
**Changes Made**: Similar to Attempts 5 and 6, with focus on using the shared utility function consistently.
|
||||
|
||||
**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip.
|
||||
|
||||
### Attempt 8: Improved Error Normalization and Deep Comparison
|
||||
|
||||
**Approach**: Modified `MemoizedCell` in `ValidationTable.tsx` to use deep comparison for error objects and improved error normalization.
|
||||
|
||||
**Changes Made**:
|
||||
```typescript
|
||||
// Create a memoized cell component
|
||||
const MemoizedCell = React.memo(({ field, value, onChange, errors, /* other props */ }) => {
|
||||
return <ValidationCell {...props} />;
|
||||
}, (prev, next) => {
|
||||
// Basic prop comparison
|
||||
if (prev.value !== next.value) return false;
|
||||
if (prev.isValidating !== next.isValidating) return false;
|
||||
if (prev.itemNumber !== next.itemNumber) return false;
|
||||
|
||||
// Deep compare errors - critical for validation display
|
||||
if (!prev.errors && next.errors) return false;
|
||||
if (prev.errors && !next.errors) return false;
|
||||
if (prev.errors && next.errors) {
|
||||
if (prev.errors.length !== next.errors.length) return false;
|
||||
|
||||
// Compare each error object
|
||||
for (let i = 0; i < prev.errors.length; i++) {
|
||||
if (prev.errors[i].message !== next.errors[i].message) return false;
|
||||
if (prev.errors[i].level !== next.errors[i].level) return false;
|
||||
if (prev.errors[i].source !== next.errors[i].source) return false;
|
||||
}
|
||||
}
|
||||
|
||||
// Compare options...
|
||||
|
||||
return true;
|
||||
});
|
||||
|
||||
// In the field columns definition:
|
||||
cell: ({ row }) => {
|
||||
const rowErrors = validationErrors.get(row.index);
|
||||
const cellErrors = rowErrors?.[fieldKey] || [];
|
||||
|
||||
// Ensure cellErrors is always an array
|
||||
const normalizedErrors = Array.isArray(cellErrors) ? cellErrors : [cellErrors];
|
||||
|
||||
return <MemoizedCell {...props} errors={normalizedErrors} />;
|
||||
}
|
||||
```
|
||||
|
||||
**Result**: Non-empty fields with validation errors still aren't displaying the alert icon with tooltip.
|
||||
|
||||
## Root Causes (Revised Hypothesis)
|
||||
|
||||
After multiple attempts, the issue appears more complex than initially thought. Possible root causes:
|
||||
|
||||
1. **Error Object Structure**: Error objects might not have the expected structure or properties
|
||||
2. **Error Propagation**: Errors might be getting filtered out before reaching cell components
|
||||
3. **Validation Rules Configuration**: Validation rules in import.tsx might be incorrectly configured
|
||||
4. **Error State Management**: Error state might not be properly updated or might be reset incorrectly
|
||||
5. **Component Rendering Logic**: Components might not re-render when validation state changes
|
||||
6. **CSS/Styling Issues**: Validation icons might be rendered but hidden due to styling issues
|
||||
7. **Validation Timing**: Validation might be happening at the wrong time or getting overridden
|
||||
138
docs/validate-table-changes-implementation-issue8.md
Normal file
138
docs/validate-table-changes-implementation-issue8.md
Normal file
@@ -0,0 +1,138 @@
|
||||
# Multiple Cell Edit Issue Implementation
|
||||
|
||||
## Issue Being Addressed
|
||||
|
||||
**Multiple Cell Edit Issue**: When you enter values in 2+ cells before validation finishes, contents from all edited cells get erased when validation finishes.
|
||||
|
||||
## Implementation Attempts
|
||||
|
||||
### Attempt 1: Fix Multiple Cell Edit Issue (First Approach)
|
||||
|
||||
**Approach**:
|
||||
- Added a tracking mechanism using a Set to keep track of cells that are currently being edited
|
||||
- Modified the `flushPendingUpdates` function to preserve values of cells being edited
|
||||
- Added cleanup of editing state after validation completes
|
||||
|
||||
**Changes Made**:
|
||||
```typescript
|
||||
// Add ref to track cells currently being edited
|
||||
const currentlyEditingCellsRef = useRef(new Set<string>());
|
||||
|
||||
// Update a row's field value
|
||||
const updateRow = useCallback((rowIndex: number, key: T, value: any) => {
|
||||
// Add this cell to currently editing cells
|
||||
const cellKey = `${rowIndex}-${key}`;
|
||||
currentlyEditingCellsRef.current.add(cellKey);
|
||||
|
||||
// ...existing code...
|
||||
|
||||
// After validation completes, remove this cell from currently editing list
|
||||
setTimeout(() => {
|
||||
currentlyEditingCellsRef.current.delete(cellKey);
|
||||
}, 100);
|
||||
}, []);
|
||||
|
||||
// Modify flushPendingUpdates to respect currently editing cells
|
||||
const flushPendingUpdates = useCallback(() => {
|
||||
// ...existing code...
|
||||
|
||||
if (dataUpdates.length > 0) {
|
||||
setData(prev => {
|
||||
// ...existing code...
|
||||
|
||||
dataUpdates.forEach((row, index) => {
|
||||
if (index < newData.length) {
|
||||
const updatedRow = { ...row };
|
||||
|
||||
// Check if any fields in this row are currently being edited
|
||||
// If so, preserve their current values in the previous data
|
||||
Object.keys(prev[index] || {}).forEach(key => {
|
||||
const cellKey = `${index}-${key}`;
|
||||
if (currentlyEditingCellsRef.current.has(cellKey)) {
|
||||
// Keep the value from the previous state for this field
|
||||
updatedRow[key] = prev[index][key];
|
||||
}
|
||||
});
|
||||
|
||||
newData[index] = updatedRow;
|
||||
}
|
||||
});
|
||||
|
||||
return newData;
|
||||
});
|
||||
}
|
||||
}, []);
|
||||
```
|
||||
|
||||
**Result**:
|
||||
- Slight improvement - the first value entered was saved, but any subsequent values still got erased
|
||||
|
||||
### Attempt 2: Fix Multiple Cell Edit Issue (Second Approach)
|
||||
|
||||
**Approach**:
|
||||
- Completely revised the cell editing tracking system
|
||||
- Used a Map with timestamps to track editing cells more accurately
|
||||
- Added proper Promise-based tracking for cell validation
|
||||
- Increased timeout from 100ms to 1000ms
|
||||
- Made cleanup more robust by checking if it's still the same editing session
|
||||
|
||||
**Changes Made**:
|
||||
```typescript
|
||||
// Add ref to track cells currently being edited with timestamps
|
||||
const currentlyEditingCellsRef = useRef(new Map<string, number>());
|
||||
|
||||
// Add ref to track validation promises
|
||||
const validationPromisesRef = useRef<Map<string, Promise<void>>>(new Map());
|
||||
|
||||
// Update a row's field value
|
||||
const updateRow = useCallback((rowIndex: number, key: T, value: any) => {
|
||||
// Mark this cell as being edited with the current timestamp
|
||||
const cellKey = `${rowIndex}-${key}`;
|
||||
currentlyEditingCellsRef.current.set(cellKey, Date.now());
|
||||
|
||||
// ...existing code...
|
||||
|
||||
// Create a validation promise
|
||||
const validationPromise = new Promise<void>((resolve) => {
|
||||
setTimeout(() => {
|
||||
try {
|
||||
validateRow(rowIndex);
|
||||
} finally {
|
||||
resolve();
|
||||
}
|
||||
}, 0);
|
||||
});
|
||||
|
||||
validationPromisesRef.current.set(cellKey, validationPromise);
|
||||
|
||||
// When validation is complete, remove from validating cells
|
||||
validationPromise.then(() => {
|
||||
// ...existing code...
|
||||
|
||||
// Keep this cell in the editing state for a longer time
|
||||
setTimeout(() => {
|
||||
if (currentlyEditingCellsRef.current.has(cellKey)) {
|
||||
currentlyEditingCellsRef.current.delete(cellKey);
|
||||
}
|
||||
}, 1000); // Keep as "editing" for 1 second
|
||||
});
|
||||
}, []);
|
||||
```
|
||||
|
||||
**Result**:
|
||||
- Worse than the first approach - now all values get erased, including the first one
|
||||
|
||||
## Root Causes (Hypothesized)
|
||||
|
||||
- The validation process might be updating the entire data state, causing race conditions with cell edits
|
||||
- The timing of validation completions might be problematic
|
||||
- State updates might be happening in a way that overwrites user changes
|
||||
- The cell state tracking system is not robust enough to prevent overwrites
|
||||
|
||||
## Next Steps
|
||||
|
||||
The issue requires a more fundamental approach than just tweaking the editing logic. We need to:
|
||||
|
||||
1. Implement a more robust state management system for cell edits that can survive validation cycles
|
||||
2. Consider disabling validation during active editing
|
||||
3. Implement a proper "dirty state" tracking system for cells
|
||||
Reference in New Issue
Block a user