Address large data table review findings

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@netdata/netdata-ui",
-  "version": "5.5.0",
+  "version": "5.5.2",
   "description": "netdata UI kit",
   "main": "dist/index.js",
   "module": "dist/es6/index.js",

src/components/table/body/index.js

@@ -14,7 +14,7 @@ import { useTableState } from "../provider"
 import Row from "./row"
 import Header from "./header"
 import RowPlaceholdersRenderer from "./rowPLaceholdersRenderer"
-import { getVirtualWindowRange } from "../largeData"
+import { normalizeWindowRange } from "../largeData"
 import { createRowMountController } from "./rowMountController"
 import { measureTableElement } from "./measureElement"
 import OverflowTooltip from "../components/overflowTooltip"
@@ -23,34 +23,32 @@ const deferredRowScrollResetDelayMs = 50
 
 const noop = () => {}
 
-const DeferredRow = memo(
-  ({ children, controller, RowPlaceholder, index, placeholderSize }) => {
-    const [ready, setReady] = useState(false)
+const getVirtualWindowRange = (virtualItems, count) => {
+  let startIndex
+  let endIndex
 
-    useEffect(() => controller.schedule(() => setReady(true)), [controller])
+  virtualItems.forEach(item => {
+    if (item.index === 0) return
+    const index = item.index - 1
+    startIndex = startIndex === undefined ? index : Math.min(startIndex, index)
+    endIndex = endIndex === undefined ? index + 1 : Math.max(endIndex, index + 1)
+  })
 
-    if (ready) return children
-    return (
-      <div
-        aria-hidden
-        style={{ height: `${placeholderSize}px`, overflow: "hidden", width: "100%" }}
-      >
-        {RowPlaceholder ? <RowPlaceholder index={index} /> : null}
-      </div>
-    )
-  },
-  (previous, next) =>
-    previous.controller === next.controller &&
-    previous.rowKey === next.rowKey &&
-    previous.rowOriginal === next.rowOriginal &&
-    previous.RowPlaceholder === next.RowPlaceholder &&
-    previous.RowWrapper === next.RowWrapper &&
-    previous.GroupRow === next.GroupRow &&
-    previous.onClickRow === next.onClickRow &&
-    previous.directCellContent === next.directCellContent &&
-    previous.placeholderSize === next.placeholderSize &&
-    previous.index === next.index
-)
+  return normalizeWindowRange({ startIndex, endIndex }, count)
+}
+
+const DeferredRow = ({ children, controller, RowPlaceholder, index, placeholderSize }) => {
+  const [ready, setReady] = useState(false)
+
+  useEffect(() => controller.schedule(() => setReady(true)), [controller])
+
+  if (ready) return children
+  return (
+    <div aria-hidden style={{ height: `${placeholderSize}px`, overflow: "hidden", width: "100%" }}>
+      {RowPlaceholder ? <RowPlaceholder index={index} /> : null}
+    </div>
+  )
+}
 
 const rerenderSelector = state => ({
   sorting: state.sorting,
@@ -175,9 +173,14 @@ const Body = memo(
 
       publishedWindowRef.current = windowRange
       onWindowChange(windowRange)
-    }, [deferRowMount, nextWindowRange, onWindowChange, rowVirtualizer.isScrolling])
+    }, [
+      deferRowMount,
+      nextWindowRange?.startIndex,
+      nextWindowRange?.endIndex,
+      onWindowChange,
+      rowVirtualizer.isScrolling,
+    ])
 
-    // index 0 is reserved for the sticky header (see count = rows + 1 and rangeExtractor above)
     const firstVirtualDataIndex = virtualRows[1]?.index ?? 1
     const lastVirtualDataIndex = virtualRows[virtualRows.length - 1]?.index ?? 0
 
@@ -187,14 +190,12 @@ const Body = memo(
       const firstDataIndex = 1
       const lastDataIndex = dataRowCount
 
-      // "before" = rows before the virtual window; capped to placeholdersLength when provided
       const beforeEnd = firstVirtualDataIndex
       const beforeStart =
         placeholdersLength != null
           ? Math.max(firstDataIndex, beforeEnd - placeholdersLength)
           : firstDataIndex
 
-      // "after" = rows after the virtual window; capped to placeholdersLength when provided
       const afterStart = lastVirtualDataIndex + 1
       const afterEnd =
         placeholdersLength != null
@@ -328,13 +329,7 @@ const Body = memo(
                 ) : deferRowMount && row ? (
                   <DeferredRow
                     controller={rowMountController}
-                    rowKey={virtualRow.key}
-                    rowOriginal={row.original}
                     RowPlaceholder={DeferredRowPlaceholder || RowPlaceholder}
-                    RowWrapper={RowWrapper}
-                    GroupRow={rest.GroupRow}
-                    onClickRow={rest.onClickRow}
-                    directCellContent={rest.directCellContent}
                     index={virtualRow.index - 1}
                     placeholderSize={virtualRow.size ?? estimateSize(virtualRow.index)}
                   >

src/components/table/body/index.test.js

@@ -6,20 +6,26 @@ import { useVirtualizer } from "@tanstack/react-virtual"
 
 jest.mock("@tanstack/react-virtual", () => ({
   defaultRangeExtractor: jest.fn(() => []),
-  useVirtualizer: jest.fn(() => ({
-    getTotalSize: () => 0,
-    getVirtualItems: () => [],
-    measureElement: jest.fn(),
-    measurementsCache: [],
-  })),
+  useVirtualizer: jest.fn(),
 }))
 
 jest.mock("./row", () => ({ row }) => (
   <div data-testid="row-content" data-row-value={row.original?.value} />
 ))
 jest.mock("./header", () => () => <div data-testid="header-content" />)
 
+const createDefaultVirtualizer = () => ({
+  getTotalSize: () => 0,
+  getVirtualItems: () => [],
+  measureElement: jest.fn(),
+  measurementsCache: [],
+})
+
 describe("Table Body large-data mode", () => {
+  beforeEach(() => {
+    useVirtualizer.mockImplementation(createDefaultVirtualizer)
+  })
+
   it("keeps virtual measurement callbacks stable across equivalent renders", () => {
     const largeDataSource = {
       getRowCount: () => 50_000,

src/components/table/body/measureElement.js

@@ -1,38 +1,19 @@
+import { measureElement } from "@tanstack/react-virtual"
+
 const getCachedSize = (element, instance) => {
   const index = instance.indexFromElement(element)
   const key = instance.options.getItemKey(index)
 
-  return {
-    cached: instance.itemSizeCache.get(key),
-    estimated: instance.options.estimateSize(index),
-  }
-}
-
-const getObservedSize = (entry, horizontal) => {
-  const borderBoxSize = entry?.borderBoxSize
-  const box = Array.isArray(borderBoxSize) ? borderBoxSize[0] : borderBoxSize
-
-  return box?.[horizontal ? "inlineSize" : "blockSize"]
+  return instance.itemSizeCache.get(key)
 }
 
 export const measureTableElement = (element, entry, instance) => {
-  const horizontal = instance.options.horizontal
-
-  if (instance.options.useCachedMeasurements) {
-    const { cached, estimated } = getCachedSize(element, instance)
-    return cached ?? estimated
-  }
-
-  const observedSize = getObservedSize(entry, horizontal)
-  if (observedSize) return observedSize
-
   if (!entry) {
-    const { cached } = getCachedSize(element, instance)
+    const cached = getCachedSize(element, instance)
     if (cached !== undefined) return cached
   }
 
-  const rect = element.getBoundingClientRect()
-  const measuredSize = horizontal ? rect.width : rect.height
+  const measuredSize = measureElement(element, entry, instance)
 
   return measuredSize || instance.options.estimateSize(instance.indexFromElement(element))
 }

src/components/table/body/measureElement.test.js

@@ -7,40 +7,37 @@ const createInstance = ({ cached, estimate = 35, horizontal = false } = {}) => (
     estimateSize: jest.fn(() => estimate),
     getItemKey: jest.fn(index => `row-${index}`),
     horizontal,
-    useCachedMeasurements: false,
   },
 })
 
 describe("measureTableElement", () => {
-  it("preserves a fractional ResizeObserver border-box height", () => {
-    const element = { getBoundingClientRect: jest.fn() }
+  it("uses TanStack's rounded ResizeObserver border-box height", () => {
+    const element = { offsetHeight: 0 }
     const instance = createInstance()
     const entry = { borderBoxSize: [{ blockSize: 34.4, inlineSize: 200.8 }] }
 
-    expect(measureTableElement(element, entry, instance)).toBe(34.4)
-    expect(element.getBoundingClientRect).not.toHaveBeenCalled()
+    expect(measureTableElement(element, entry, instance)).toBe(34)
   })
 
-  it("preserves a fractional bounding-client-rect fallback", () => {
-    const element = { getBoundingClientRect: jest.fn(() => ({ height: 52.8, width: 400.8 })) }
+  it("uses TanStack's offset-size fallback", () => {
+    const element = { offsetHeight: 53 }
     const instance = createInstance()
 
-    expect(measureTableElement(element, undefined, instance)).toBe(52.8)
+    expect(measureTableElement(element, undefined, instance)).toBe(53)
   })
 
   it("reuses a cached measurement without forcing layout", () => {
-    const element = { getBoundingClientRect: jest.fn() }
+    const element = { offsetHeight: 53 }
     const instance = createInstance({ cached: 44.8 })
 
     expect(measureTableElement(element, undefined, instance)).toBe(44.8)
-    expect(element.getBoundingClientRect).not.toHaveBeenCalled()
   })
 
-  it("measures fractional width for a horizontal virtualizer", () => {
-    const element = { getBoundingClientRect: jest.fn() }
+  it("uses TanStack's rounded width for a horizontal virtualizer", () => {
+    const element = { offsetWidth: 0 }
     const instance = createInstance({ horizontal: true })
-    const entry = { borderBoxSize: { blockSize: 34.4, inlineSize: 200.8 } }
+    const entry = { borderBoxSize: [{ blockSize: 34.4, inlineSize: 200.8 }] }
 
-    expect(measureTableElement(element, entry, instance)).toBe(200.8)
+    expect(measureTableElement(element, entry, instance)).toBe(201)
   })
 })