fix: refactoring

src/components/common/TreeExplorer.vue

@@ -63,7 +63,13 @@ import type {
 } from '@/types/treeExplorerTypes'
 import { combineTrees } from '@/utils/treeUtil'
 import type { WindowRange } from '@/utils/virtualListUtils'
-import { applyWindow, createInitialWindowRange } from '@/utils/virtualListUtils'
+import {
+  applyWindow,
+  calculateSpacerHeightsVariable,
+  calculateWindowRangeByHeights,
+  createInitialWindowRange,
+  mergeWindowRange
+} from '@/utils/virtualListUtils'
 
 const expandedKeys = defineModel<Record<string, boolean>>('expandedKeys', {
   required: true
@@ -117,6 +123,9 @@ const viewRows = computed(() =>
 const bufferRows = computed(() => Math.max(1, Math.floor(viewRows.value / 3)))
 const windowSize = computed(() => viewRows.value + bufferRows.value * 2)
 
+const isNodeExpanded = (node: RenderedTreeExplorerNode): boolean =>
+  !!(node.children && !node.leaf && expandedKeys.value?.[node.key])
+
 // Calculate positions for all nodes in the tree
 const calculateNodePositions = (
   root: RenderedTreeExplorerNode
@@ -128,8 +137,8 @@ const calculateNodePositions = (
     nodePositions.set(node.key, currentPos)
     currentPos += DEFAULT_NODE_HEIGHT
 
-    if (node.children && !node.leaf && expandedKeys.value?.[node.key]) {
-      node.children.forEach(traverse)
+    if (isNodeExpanded(node)) {
+      node.children!.forEach(traverse)
     }
   }
 
@@ -138,12 +147,12 @@ const calculateNodePositions = (
 }
 
 const getFullNodeHeight = (node: RenderedTreeExplorerNode): number => {
-  if (!node.children || node.leaf || !expandedKeys.value?.[node.key]) {
+  if (!isNodeExpanded(node)) {
     return DEFAULT_NODE_HEIGHT
   }
   return (
     DEFAULT_NODE_HEIGHT +
-    node.children.reduce((sum, child) => sum + getFullNodeHeight(child), 0)
+    node.children!.reduce((sum, child) => sum + getFullNodeHeight(child), 0)
   )
 }
 
@@ -154,73 +163,36 @@ const calculateNodeWindowRange = (
   scrollBottom: number,
   bufferHeight: number
 ): WindowRange | null => {
-  if (
-    !node.children ||
-    node.leaf ||
-    !expandedKeys.value?.[node.key] ||
-    node.children.length === 0
-  ) {
-    return null
-  }
+  if (!isNodeExpanded(node)) return null
+
+  const children = node.children!
+  if (!children.length) return null
 
-  const totalChildren = node.children.length
   const nodeStart = nodePositions.get(node.key) ?? 0
   const childrenStart = nodeStart + DEFAULT_NODE_HEIGHT
-
-  const lastChild = node.children[totalChildren - 1]
+  const lastChild = children[children.length - 1]
   const lastChildStart = nodePositions.get(lastChild.key) ?? childrenStart
   const childrenEnd = lastChildStart + getFullNodeHeight(lastChild)
 
-  const scrollTopWithBuffer = scrollTop - bufferHeight
-  const scrollBottomWithBuffer = scrollBottom + bufferHeight
-
-  // Quick checks for nodes outside viewport
-  if (childrenEnd < scrollTopWithBuffer) {
-    return {
-      start: Math.max(0, totalChildren - windowSize.value),
-      end: totalChildren
-    }
-  }
-
-  if (childrenStart > scrollBottomWithBuffer) {
-    return { start: 0, end: Math.min(windowSize.value, totalChildren) }
-  }
-
-  // Find visible range by iterating children
-  let startIndex = 0
-  let endIndex = totalChildren
-
-  for (let i = 0; i < totalChildren; i++) {
-    const child = node.children[i]
-    const childStart = nodePositions.get(child.key) ?? 0
-    const childEnd = childStart + getFullNodeHeight(child)
-
-    if (childEnd < scrollTopWithBuffer) {
-      startIndex = i + 1
-    }
-    if (childStart <= scrollBottomWithBuffer) {
-      endIndex = i + 1
-    }
-  }
-
-  // Apply buffer and ensure minimum window size
-  startIndex = Math.max(0, startIndex - bufferRows.value)
-  endIndex = Math.min(
-    totalChildren,
-    Math.max(endIndex + bufferRows.value, startIndex + windowSize.value)
-  )
-
-  return { start: startIndex, end: endIndex }
+  return calculateWindowRangeByHeights({
+    items: children,
+    listStart: childrenStart,
+    listEnd: childrenEnd,
+    scrollTop,
+    scrollBottom,
+    bufferHeight,
+    bufferRows: bufferRows.value,
+    windowSize: windowSize.value,
+    getItemStart: (child) => nodePositions.get(child.key) ?? 0,
+    getItemHeight: getFullNodeHeight
+  })
 }
 
 const getTreeNodeIcon = (node: TreeExplorerNode): string => {
   const customIcon = node.getIcon?.() ?? node.icon
   if (customIcon) return customIcon
-
   if (node.leaf) return 'pi pi-file'
-
-  const isExpanded = expandedKeys.value?.[node.key] ?? false
-  return isExpanded ? 'pi pi-folder-open' : 'pi pi-folder'
+  return expandedKeys.value?.[node.key] ? 'pi pi-folder-open' : 'pi pi-folder'
 }
 
 const fillNodeInfo = (node: TreeExplorerNode): RenderedTreeExplorerNode => {
@@ -255,46 +227,6 @@ const nodeKeyMap = computed<Record<string, RenderedTreeExplorerNode>>(() => {
   return map
 })
 
-const mergeRanges = (
-  existing: WindowRange | undefined,
-  calculated: WindowRange,
-  totalChildren: number
-): { range: WindowRange; changed: boolean } => {
-  if (!existing) {
-    return { range: calculated, changed: true }
-  }
-
-  const buffer = bufferRows.value
-  const maxWindow = windowSize.value * 2
-
-  const updateStart =
-    calculated.start < existing.start ||
-    (existing.start > 0 && calculated.start > existing.start + buffer)
-  const updateEnd =
-    calculated.end > existing.end ||
-    (existing.end < totalChildren && calculated.end < existing.end - buffer)
-
-  let newStart = updateStart ? calculated.start : existing.start
-  let newEnd = updateEnd ? calculated.end : existing.end
-
-  // Limit maximum window size
-  if (newEnd - newStart > maxWindow) {
-    if (updateStart) {
-      newEnd = Math.min(totalChildren, newStart + maxWindow)
-    } else {
-      newStart = Math.max(0, newEnd - maxWindow)
-    }
-  }
-
-  const changed =
-    updateStart ||
-    updateEnd ||
-    newStart !== existing.start ||
-    newEnd !== existing.end
-
-  return { range: { start: newStart, end: newEnd }, changed }
-}
-
 const updateVisibleParentRanges = () => {
   if (!containerHeight.value || !renderedRoot.value.children) {
     return
@@ -307,16 +239,15 @@ const updateVisibleParentRanges = () => {
   const currentRanges = parentNodeWindowRanges.value
   const newRanges: Record<string, WindowRange> = {}
   let hasChanges = false
+  const mergeOptions = {
+    bufferRows: bufferRows.value,
+    windowSize: windowSize.value
+  }
 
   const processNode = (node: RenderedTreeExplorerNode) => {
-    if (
-      !node.children ||
-      node.leaf ||
-      !expandedKeys.value?.[node.key]
-    ) {
-      return
-    }
+    if (!isNodeExpanded(node)) return
 
+    const children = node.children!
     const calculated = calculateNodeWindowRange(
       node,
       nodePositions,
@@ -326,16 +257,16 @@ const updateVisibleParentRanges = () => {
     )
 
     if (calculated) {
-      const { range, changed } = mergeRanges(
+      const { range, changed } = mergeWindowRange(
         currentRanges[node.key],
         calculated,
-        node.children.length
+        { ...mergeOptions, totalChildren: children.length }
       )
       newRanges[node.key] = range
       if (changed) hasChanges = true
     }
 
-    node.children.forEach(processNode)
+    children.forEach(processNode)
   }
 
   renderedRoot.value.children.forEach(processNode)
@@ -348,54 +279,43 @@ const updateVisibleParentRanges = () => {
   }
 }
 
-watch([scrollY, containerHeight], updateVisibleParentRanges, { immediate: true })
+watch([scrollY, containerHeight], updateVisibleParentRanges, {
+  immediate: true
+})
 watch(expandedKeys, updateVisibleParentRanges, { deep: true })
 
-const displayRoot = computed<RenderedTreeExplorerNode>(() => ({
-  ...renderedRoot.value,
-  children: (renderedRoot.value.children || []).map((node) =>
-    applyWindow(node, parentNodeWindowRanges.value, windowSize.value)
-  )
-}))
-
-const calculateRealSpacerHeights = (
-  originalChildren: RenderedTreeExplorerNode[],
-  range: WindowRange
-): { topSpacer: number; bottomSpacer: number } => {
-  const topSpacer = originalChildren
-    .slice(0, range.start)
-    .reduce((sum, child) => sum + getFullNodeHeight(child), 0)
-
-  const bottomSpacer = originalChildren
-    .slice(range.end)
-    .reduce((sum, child) => sum + getFullNodeHeight(child), 0)
-
-  return { topSpacer, bottomSpacer }
-}
+const displayRoot = computed<RenderedTreeExplorerNode>(() => {
+  const root = renderedRoot.value
+  if (!root.children) return root
+  return {
+    ...root,
+    children: root.children.map((node) =>
+      applyWindow(node, parentNodeWindowRanges.value, windowSize.value)
+    )
+  }
+})
 
 const getNodeChildrenStyle = (node: RenderedTreeExplorerNode) => {
   const baseStyle = { class: 'virtual-node-children' }
 
-  if (
-    !node ||
-    node.leaf ||
-    !expandedKeys.value?.[node.key]
-  ) {
+  if (!node || !isNodeExpanded(node)) {
     return baseStyle
   }
 
   const originalNode = nodeKeyMap.value[node.key]
-  if (!originalNode?.children?.length) {
+  const children = originalNode?.children
+  if (!children?.length) {
     return baseStyle
   }
 
   const range =
     parentNodeWindowRanges.value[node.key] ??
-    createInitialWindowRange(originalNode.children.length, windowSize.value)
+    createInitialWindowRange(children.length, windowSize.value)
 
-  const { topSpacer, bottomSpacer } = calculateRealSpacerHeights(
-    originalNode.children,
-    range
+  const { topSpacer, bottomSpacer } = calculateSpacerHeightsVariable(
+    children,
+    range,
+    getFullNodeHeight
   )
 
   return {
@@ -421,7 +341,7 @@ const onNodeContentClick = async (
       node.handleError
     )()
   }
-  
+
   emit('nodeClick', node, e)
 }
 
@@ -433,6 +353,23 @@ const extraMenuItems = computed(() => {
     : contextMenuItems
 })
 
+const wrapCommandWithErrorHandler = (
+  command: (event: MenuItemCommandEvent) => void,
+  { isAsync = false }: { isAsync: boolean }
+):
+  | ((event: MenuItemCommandEvent) => void)
+  | ((event: MenuItemCommandEvent) => Promise<void>) => {
+  return isAsync
+    ? errorHandling.wrapWithErrorHandlingAsync(
+        command as (event: MenuItemCommandEvent) => Promise<void>,
+        menuTargetNode.value?.handleError
+      )
+    : errorHandling.wrapWithErrorHandling(
+        command,
+        menuTargetNode.value?.handleError
+      )
+}
+
 const handleNodeLabelEdit = async (
   node: RenderedTreeExplorerNode,
   newName: string
@@ -498,23 +435,6 @@ const handleContextMenu = (e: MouseEvent, node: RenderedTreeExplorerNode) => {
   }
 }
 
-const wrapCommandWithErrorHandler = (
-  command: (event: MenuItemCommandEvent) => void,
-  { isAsync = false }: { isAsync: boolean }
-):
-  | ((event: MenuItemCommandEvent) => void)
-  | ((event: MenuItemCommandEvent) => Promise<void>) => {
-  return isAsync
-    ? errorHandling.wrapWithErrorHandlingAsync(
-        command as (event: MenuItemCommandEvent) => Promise<void>,
-        menuTargetNode.value?.handleError
-      )
-    : errorHandling.wrapWithErrorHandling(
-        command,
-        menuTargetNode.value?.handleError
-      )
-}
-
 defineExpose({
   /**
    * The command to add a folder to a node via the context menu
@@ -525,7 +445,7 @@ defineExpose({
     if (targetNode) {
       addFolderCommand(targetNode)
     }
-  },
+  }
 })
 </script>
 

src/utils/__tests__/virtualListUtils.test.ts

@@ -5,8 +5,10 @@ import type { RenderedTreeExplorerNode } from '@/types/treeExplorerTypes'
 import type { WindowRange } from '../virtualListUtils'
 import {
   applyWindow,
-  calculateSpacerHeights,
-  createInitialWindowRange
+  calculateSpacerHeightsVariable,
+  calculateWindowRangeByHeights,
+  createInitialWindowRange,
+  mergeWindowRange
 } from '../virtualListUtils'
 
 describe('virtualListUtils', () => {
@@ -27,43 +29,6 @@ describe('virtualListUtils', () => {
     })
   })
 
-  describe('calculateSpacerHeights', () => {
-    it('calculates correct spacer heights', () => {
-      const range: WindowRange = { start: 20, end: 80 }
-      const result = calculateSpacerHeights(100, range, 28)
-      expect(result).toEqual({
-        topSpacer: 20 * 28,
-        bottomSpacer: 20 * 28
-      })
-    })
-
-    it('returns zero spacers when window covers all children', () => {
-      const range: WindowRange = { start: 0, end: 50 }
-      const result = calculateSpacerHeights(50, range, 28)
-      expect(result).toEqual({
-        topSpacer: 0,
-        bottomSpacer: 0
-      })
-    })
-
-    it('handles window at start', () => {
-      const range: WindowRange = { start: 0, end: 60 }
-      const result = calculateSpacerHeights(100, range, 28)
-      expect(result).toEqual({
-        topSpacer: 0,
-        bottomSpacer: 40 * 28
-      })
-    })
-
-    it('handles window at end', () => {
-      const range: WindowRange = { start: 40, end: 100 }
-      const result = calculateSpacerHeights(100, range, 28)
-      expect(result).toEqual({
-        topSpacer: 40 * 28,
-        bottomSpacer: 0
-      })
-    })
-  })
   describe('applyWindow', () => {
     const createMockNode = (
       key: string,
@@ -134,4 +99,102 @@ describe('virtualListUtils', () => {
       expect(result.children).toHaveLength(30)
     })
   })
+
+  describe('mergeWindowRange', () => {
+    it('returns calculated when existing is undefined', () => {
+      const result = mergeWindowRange(
+        undefined,
+        { start: 10, end: 20 },
+        {
+          bufferRows: 2,
+          windowSize: 10,
+          totalChildren: 100
+        }
+      )
+      expect(result).toEqual({ range: { start: 10, end: 20 }, changed: true })
+    })
+
+    it('keeps existing when calculated is within buffer', () => {
+      const result = mergeWindowRange(
+        { start: 10, end: 30 },
+        { start: 11, end: 29 },
+        { bufferRows: 5, windowSize: 10, totalChildren: 100 }
+      )
+      expect(result.changed).toBe(false)
+      expect(result.range).toEqual({ start: 10, end: 30 })
+    })
+
+    it('expands when calculated exceeds existing', () => {
+      const result = mergeWindowRange(
+        { start: 10, end: 30 },
+        { start: 5, end: 40 },
+        { bufferRows: 2, windowSize: 10, totalChildren: 100 }
+      )
+      expect(result.changed).toBe(true)
+      expect(result.range).toEqual({ start: 5, end: 40 })
+    })
+  })
+
+  describe('calculateSpacerHeightsVariable', () => {
+    it('calculates spacers using variable item heights', () => {
+      const items = [10, 20, 30, 40, 50]
+      const range: WindowRange = { start: 1, end: 4 } // visible: 20,30,40
+      const result = calculateSpacerHeightsVariable(items, range, (n) => n)
+      expect(result).toEqual({ topSpacer: 10, bottomSpacer: 50 })
+    })
+  })
+
+  describe('calculateWindowRangeByHeights', () => {
+    it('returns start window when list is below viewport', () => {
+      const items = Array.from({ length: 100 }, (_, i) => i)
+      const range = calculateWindowRangeByHeights({
+        items,
+        listStart: 1000,
+        listEnd: 2000,
+        scrollTop: 0,
+        scrollBottom: 300,
+        bufferHeight: 50,
+        bufferRows: 2,
+        windowSize: 10,
+        getItemStart: (n) => n * 10,
+        getItemHeight: () => 10
+      })
+      expect(range).toEqual({ start: 0, end: 10 })
+    })
+
+    it('returns end window when list is above viewport', () => {
+      const items = Array.from({ length: 100 }, (_, i) => i)
+      const range = calculateWindowRangeByHeights({
+        items,
+        listStart: 0,
+        listEnd: 500,
+        scrollTop: 2000,
+        scrollBottom: 2300,
+        bufferHeight: 50,
+        bufferRows: 2,
+        windowSize: 10,
+        getItemStart: (n) => n * 10,
+        getItemHeight: () => 10
+      })
+      expect(range).toEqual({ start: 90, end: 100 })
+    })
+
+    it('computes window around viewport with buffer and min size', () => {
+      const items = Array.from({ length: 100 }, (_, i) => i)
+      const range = calculateWindowRangeByHeights({
+        items,
+        listStart: 0,
+        listEnd: 2000,
+        scrollTop: 200,
+        scrollBottom: 260,
+        bufferHeight: 0,
+        bufferRows: 2,
+        windowSize: 10,
+        getItemStart: (n) => n * 10,
+        getItemHeight: () => 10
+      })
+      expect(range.end - range.start).toBeGreaterThanOrEqual(10)
+      expect(range.start).toBeLessThanOrEqual(20)
+    })
+  })
 })

src/utils/virtualListUtils.ts

@@ -5,6 +5,123 @@ export interface WindowRange {
   end: number
 }
 
+export function mergeWindowRange(
+  existing: WindowRange | undefined,
+  calculated: WindowRange,
+  {
+    bufferRows,
+    windowSize,
+    totalChildren,
+    maxWindowSize = windowSize * 2
+  }: {
+    bufferRows: number
+    windowSize: number
+    totalChildren: number
+    maxWindowSize?: number
+  }
+): { range: WindowRange; changed: boolean } {
+  if (!existing) {
+    return { range: calculated, changed: true }
+  }
+
+  const updateStart =
+    calculated.start < existing.start ||
+    (existing.start > 0 && calculated.start > existing.start + bufferRows)
+  const updateEnd =
+    calculated.end > existing.end ||
+    (existing.end < totalChildren && calculated.end < existing.end - bufferRows)
+
+  let start = updateStart ? calculated.start : existing.start
+  let end = updateEnd ? calculated.end : existing.end
+
+  if (end - start > maxWindowSize) {
+    if (updateStart) {
+      end = Math.min(totalChildren, start + maxWindowSize)
+    } else {
+      start = Math.max(0, end - maxWindowSize)
+    }
+  }
+
+  const changed =
+    updateStart || updateEnd || start !== existing.start || end !== existing.end
+
+  return { range: { start, end }, changed }
+}
+
+export function calculateSpacerHeightsVariable<T>(
+  items: T[],
+  range: WindowRange,
+  getHeight: (item: T) => number
+): { topSpacer: number; bottomSpacer: number } {
+  const topSpacer = items
+    .slice(0, range.start)
+    .reduce((sum, item) => sum + getHeight(item), 0)
+
+  const bottomSpacer = items
+    .slice(range.end)
+    .reduce((sum, item) => sum + getHeight(item), 0)
+
+  return { topSpacer, bottomSpacer }
+}
+
+export function calculateWindowRangeByHeights<T>({
+  items,
+  listStart,
+  listEnd,
+  scrollTop,
+  scrollBottom,
+  bufferHeight,
+  bufferRows,
+  windowSize,
+  getItemStart,
+  getItemHeight
+}: {
+  items: T[]
+  listStart: number
+  listEnd: number
+  scrollTop: number
+  scrollBottom: number
+  bufferHeight: number
+  bufferRows: number
+  windowSize: number
+  getItemStart: (item: T) => number
+  getItemHeight: (item: T) => number
+}): WindowRange {
+  const total = items.length
+  if (total === 0) return { start: 0, end: 0 }
+
+  const scrollTopWithBuffer = scrollTop - bufferHeight
+  const scrollBottomWithBuffer = scrollBottom + bufferHeight
+
+  // Quick checks for lists outside viewport
+  if (listEnd < scrollTopWithBuffer) {
+    return { start: Math.max(0, total - windowSize), end: total }
+  }
+
+  if (listStart > scrollBottomWithBuffer) {
+    return { start: 0, end: Math.min(windowSize, total) }
+  }
+
+  // Find visible range by iterating items
+  let start = 0
+  let end = total
+
+  for (let i = 0; i < total; i++) {
+    const item = items[i]
+    const itemStart = getItemStart(item)
+    const itemEnd = itemStart + getItemHeight(item)
+
+    if (itemEnd < scrollTopWithBuffer) start = i + 1
+    if (itemStart <= scrollBottomWithBuffer) end = i + 1
+  }
+
+  // Apply buffer and ensure minimum window size
+  start = Math.max(0, start - bufferRows)
+  end = Math.min(total, Math.max(end + bufferRows, start + windowSize))
+
+  return { start, end }
+}
+
 /**
  * Apply a sliding window to limit visible children of a node
  * @param node - The node to apply the window to
@@ -21,11 +138,9 @@ export function applyWindow(
     return node
   }
 
-  const totalChildren = node.children.length
-  const range = windowRanges[node.key] ?? {
-    start: 0,
-    end: Math.min(windowSize, totalChildren)
-  }
+  const range =
+    windowRanges[node.key] ??
+    createInitialWindowRange(node.children.length, windowSize)
 
   // Recursively apply window to children
   const windowedChildren = node.children
@@ -38,23 +153,6 @@ export function applyWindow(
   }
 }
 
-/**
- * Calculate spacer heights for a node's children
- * @param totalChildren - Total number of children
- * @param range - Current window range
- * @param nodeHeight - Height of each node in pixels
- * @returns Top and bottom spacer heights
- */
-export function calculateSpacerHeights(
-  totalChildren: number,
-  range: WindowRange,
-  nodeHeight: number
-): { topSpacer: number; bottomSpacer: number } {
-  const topSpacer = range.start * nodeHeight
-  const bottomSpacer = Math.max(0, totalChildren - range.end) * nodeHeight
-  return { topSpacer, bottomSpacer }
-}
-
 /**
  * Create initial window range for a node
  * @param totalChildren - Total number of children