fix: prevent column width changes during virtualized table scrolling

[hkdeman]

Summary

Fixed the issue where column widths were changing during scrolling in the virtualized table component.

Changes

• Apply fixed widths to body cells during initial sync for virtualized tables
• Disconnect ResizeObserver from body cells after initial sync to prevent re-calculation
• Ensures consistent column widths while scrolling through virtualized rows

Closes #23

🤖 Generated with Claude Code

[jazzberry-ai]

Bug Report

Name	Severity	Example test case	Description
ResizeObserver disconnection delay	Low		The ResizeObserver disconnection logic might run one render cycle too late, potentially leading to unnecessary ResizeObserver connections.
Potential race condition in width synchronization	Medium	Rapidly resize the window while a virtualized table is rendering.	The syncWidths function might use incorrect header cell widths if they are still being resized when the function is called. Modified to use getBoundingClientRect() to get rendered widths.
Inflexible column width management	Low		The minWidth and maxWidth properties are always set, potentially limiting the flexibility of the table. Modified to only set these properties if a width is explicitly provided in the column definition.

_{Comments? Email us.}

[claude]

Pull Request Review

Thank you for addressing issue #23! The fix correctly addresses the column width instability during virtualized table scrolling. Here's my detailed review:

✅ Strengths

1. Targeted Solution: The fix appropriately distinguishes between virtualized and non-virtualized tables, applying width constraints only where needed.

2. Correct Root Cause Fix: By applying fixed widths to body cells during initial sync and disconnecting the ResizeObserver afterward, you've addressed the core issue of width recalculation during scrolling.

3. Minimal Impact: The changes are surgical and don't affect the existing non-virtualized table behavior.

⚠️ Issues & Concerns

1. Logic Bug in ResizeObserver Cleanup (Critical)

Location: src/components/ui/table.tsx:469-473

if (isVirtualizedRef.current && widthSyncedRef.current) {
  const { bodyCells } = getCells()
  bodyCells.forEach((cell) => resizeObserver.unobserve(cell))
}

Problem: This cleanup code is inside the ResizeObserver callback and only runs AFTER widthSyncedRef.current becomes true. However, widthSyncedRef.current is set to true at line 448 (in syncWidths()), which happens BEFORE the ResizeObserver callback is triggered. This means the condition isVirtualizedRef.current && widthSyncedRef.current will never be true at this point in the code flow.

Expected behavior: The cleanup should happen after the first sync, but the current placement makes it unreachable.

Suggested fix: Move the cleanup outside the ResizeObserver callback or restructure the logic:

const resizeObserver = new ResizeObserver(() => {
  if (!isVirtualizedRef.current || !widthSyncedRef.current) {
    syncWidths()
  }
})

// Observe initially
const { bodyCells } = getCells()
bodyCells.forEach((cell) => resizeObserver.observe(cell))

// For virtualized tables, disconnect after initial sync
if (isVirtualizedRef.current) {
  requestAnimationFrame(() => {
    if (widthSyncedRef.current) {
      bodyCells.forEach((cell) => resizeObserver.unobserve(cell))
    }
  })
}

2. Race Condition Risk

Location: src/components/ui/table.tsx:460-463

The initial syncWidths() call is wrapped in requestAnimationFrame, which means there's a timing dependency. If the ResizeObserver fires before the RAF callback completes, the virtualized table detection might not have occurred yet.

Suggested approach: Consider calling checkIfVirtualized() synchronously before setting up the ResizeObserver.

3. Window Resize Behavior Inconsistency

Location: src/components/ui/table.tsx:485-488

const onResize = () => {
  widthSyncedRef.current = false
  syncWidths()
}

When the window is resized, widthSyncedRef is reset to false, which will cause virtualized tables to re-sync widths via the ResizeObserver. This is good, but the body cell observers should be re-established after cleanup. Currently, once unobserved, they're never re-observed even on window resize.

Recommendation: Either:

• Don't disconnect observers (keep them but gate the sync logic)
• Or re-establish observers when widthSyncedRef is reset

4. Missing Null Safety

Location: src/components/ui/table.tsx:471-472

const { bodyCells } = getCells()
bodyCells.forEach((cell) => resizeObserver.unobserve(cell))

getCells() can return an empty array if the table structure isn't ready. While .forEach handles empty arrays gracefully, it's safer to check that cells exist.

📊 Test Coverage

Status: ⚠️ No test coverage for this fix

The existing test file (tests/table.test.tsx) doesn't include tests for:

• VirtualizedTableBody behavior
• Width synchronization logic
• Column width stability during scrolling

Recommendation: Add integration tests for:

it('should maintain fixed column widths during virtualized scrolling', () => {
  // Test that column widths don't change when scrolling through virtualized rows
})

it('should sync widths only once for virtualized tables', () => {
  // Test that ResizeObserver doesn't trigger repeated syncs
})

it('should re-sync widths on window resize for virtualized tables', () => {
  // Test that window resize properly updates column widths
})

🚀 Performance Considerations

Positive:

• Disconnecting ResizeObserver after initial sync reduces unnecessary observation callbacks
• Fixed widths prevent layout recalculation during scrolling

Concern:

• If the logic bug is fixed, the current approach should work well

🔒 Security

No security concerns identified. The changes are purely presentational/layout-related.

📝 Code Style & Best Practices

• ✅ Follows existing code patterns
• ✅ Proper use of refs for non-reactive state
• ✅ Clear comments explaining the purpose
• ⚠️ Could benefit from extracting virtualized table setup logic into a separate function for clarity

🎯 Recommendations Summary

Must Fix:

1. Fix the ResizeObserver cleanup logic bug (lines 469-473)
2. Add test coverage for virtualized table width behavior

Should Consider:
3. Address the window resize re-observation issue
4. Add synchronous checkIfVirtualized() call before observer setup

Overall Assessment: The approach is sound and addresses the reported issue, but the implementation has a logic bug that prevents the cleanup code from executing. Once fixed, this should work as intended. I recommend testing thoroughly with virtualized tables of varying column widths to ensure the fix works as expected.

---

🤖 Generated with Claude Code