fix: update for coderabbit

2026-01-26 10:59:53 +00:00 · 2026-01-22 18:51:16 +08:00
parent d87a2fe2f9
commit 0a5baa6256
3 changed files with 452 additions and 53 deletions
--- a/src/components/common/TreeExplorer.vue
+++ b/src/components/common/TreeExplorer.vue
@@ -65,7 +65,9 @@ import { combineTrees } from '@/utils/treeUtil'
 import type { WindowRange } from '@/utils/virtualListUtils'
 import {
  applyWindow,
+  calculateChildrenListBounds,
  calculateSpacerHeightsVariable,
+  calculateTreePositionsAndHeights,
  calculateWindowRangeByHeights,
  createInitialWindowRange,
  mergeWindowRange
@@ -76,7 +78,6 @@ const expandedKeys = defineModel<Record<string, boolean>>('expandedKeys', {
 })
 provide(InjectKeyExpandedKeys, expandedKeys)
 const selectionKeys = defineModel<Record<string, boolean>>('selectionKeys')
-// Tracks whether the caller has set the selectionKeys model.
 const storeSelectionKeys = selectionKeys.value !== undefined

 const props = defineProps<{
@@ -108,6 +109,7 @@ const menu = ref<InstanceType<typeof ContextMenu> | null>(null)
 const menuTargetNode = ref<RenderedTreeExplorerNode | null>(null)
 const renameEditingNode = ref<RenderedTreeExplorerNode | null>(null)
 const parentNodeWindowRanges = ref<Record<string, WindowRange>>({})
+const nodeHeightsCache = ref<Map<string, number>>(new Map())

 const { height: containerHeight } = useElementSize(treeContainerRef)
 const { y: scrollY } = useScroll(treeContainerRef, {
@@ -126,39 +128,21 @@ 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 = (
+const calculateNodePositionsAndHeights = (
  root: RenderedTreeExplorerNode
-): Map<string, number> => {
-  const nodePositions = new Map<string, number>()
-  let currentPos = 0
-
-  const traverse = (node: RenderedTreeExplorerNode) => {
-    nodePositions.set(node.key, currentPos)
-    currentPos += DEFAULT_NODE_HEIGHT
-
-    if (isNodeExpanded(node)) {
-      node.children!.forEach(traverse)
-    }
-  }
-
-  root.children?.forEach(traverse)
-  return nodePositions
-}
-
-const getFullNodeHeight = (node: RenderedTreeExplorerNode): number => {
-  if (!isNodeExpanded(node)) {
-    return DEFAULT_NODE_HEIGHT
-  }
-  return (
-    DEFAULT_NODE_HEIGHT +
-    node.children!.reduce((sum, child) => sum + getFullNodeHeight(child), 0)
-  )
+): { positions: Map<string, number>; heights: Map<string, number> } => {
+  return calculateTreePositionsAndHeights({
+    root,
+    itemHeight: DEFAULT_NODE_HEIGHT,
+    getChildren: (node) => node.children,
+    isExpanded: isNodeExpanded
+  })
 }

 const calculateNodeWindowRange = (
  node: RenderedTreeExplorerNode,
  nodePositions: Map<string, number>,
+  nodeHeights: Map<string, number>,
  scrollTop: number,
  scrollBottom: number,
  bufferHeight: number
@@ -168,23 +152,30 @@ const calculateNodeWindowRange = (
  const children = node.children!
  if (!children.length) return null

-  const nodeStart = nodePositions.get(node.key) ?? 0
-  const childrenStart = nodeStart + DEFAULT_NODE_HEIGHT
-  const lastChild = children[children.length - 1]
-  const lastChildStart = nodePositions.get(lastChild.key) ?? childrenStart
-  const childrenEnd = lastChildStart + getFullNodeHeight(lastChild)
+  const { listStart, listEnd } = calculateChildrenListBounds({
+    node,
+    children,
+    nodePositions,
+    nodeHeights,
+    itemHeight: DEFAULT_NODE_HEIGHT
+  })
+
+  const getItemStart = (child: RenderedTreeExplorerNode) =>
+    nodePositions.get(child.key) ?? 0
+  const getItemHeight = (child: RenderedTreeExplorerNode) =>
+    nodeHeights.get(child.key) ?? DEFAULT_NODE_HEIGHT

  return calculateWindowRangeByHeights({
    items: children,
-    listStart: childrenStart,
-    listEnd: childrenEnd,
+    listStart,
+    listEnd,
    scrollTop,
    scrollBottom,
    bufferHeight,
    bufferRows: bufferRows.value,
    windowSize: windowSize.value,
-    getItemStart: (child) => nodePositions.get(child.key) ?? 0,
-    getItemHeight: getFullNodeHeight
+    getItemStart,
+    getItemHeight
  })
 }

@@ -235,7 +226,10 @@ const updateVisibleParentRanges = () => {
  const scrollTop = scrollY.value
  const scrollBottom = scrollTop + containerHeight.value
  const bufferHeight = bufferRows.value * DEFAULT_NODE_HEIGHT
-  const nodePositions = calculateNodePositions(renderedRoot.value)
+  const { positions: nodePositions, heights: nodeHeights } =
+    calculateNodePositionsAndHeights(renderedRoot.value)
+  nodeHeightsCache.value = nodeHeights
+
  const currentRanges = parentNodeWindowRanges.value
  const newRanges: Record<string, WindowRange> = {}
  let hasChanges = false
@@ -251,30 +245,30 @@ const updateVisibleParentRanges = () => {
    const calculated = calculateNodeWindowRange(
      node,
      nodePositions,
+      nodeHeights,
      scrollTop,
      scrollBottom,
      bufferHeight
    )

-    if (calculated) {
-      const { range, changed } = mergeWindowRange(
-        currentRanges[node.key],
-        calculated,
-        { ...mergeOptions, totalChildren: children.length }
-      )
-      newRanges[node.key] = range
-      if (changed) hasChanges = true
-    }
+    if (!calculated) return
+
+    const { range, changed } = mergeWindowRange(
+      currentRanges[node.key],
+      calculated,
+      { ...mergeOptions, totalChildren: children.length }
+    )
+    newRanges[node.key] = range
+    if (changed) hasChanges = true

    children.forEach(processNode)
  }

  renderedRoot.value.children.forEach(processNode)

-  if (
-    hasChanges ||
+  const keysChanged =
    Object.keys(newRanges).length !== Object.keys(currentRanges).length
-  ) {
+  if (hasChanges || keysChanged) {
    parentNodeWindowRanges.value = newRanges
  }
 }
@@ -282,6 +276,7 @@ const updateVisibleParentRanges = () => {
 watch([scrollY, containerHeight], updateVisibleParentRanges, {
  immediate: true
 })
+watch(renderedRoot, updateVisibleParentRanges)
 watch(expandedKeys, updateVisibleParentRanges, { deep: true })

 const displayRoot = computed<RenderedTreeExplorerNode>(() => {
@@ -312,10 +307,12 @@ const getNodeChildrenStyle = (node: RenderedTreeExplorerNode) => {
    parentNodeWindowRanges.value[node.key] ??
    createInitialWindowRange(children.length, windowSize.value)

+  const getHeight = (child: RenderedTreeExplorerNode) =>
+    nodeHeightsCache.value.get(child.key) ?? DEFAULT_NODE_HEIGHT
  const { topSpacer, bottomSpacer } = calculateSpacerHeightsVariable(
    children,
    range,
-    getFullNodeHeight
+    getHeight
  )

  return {
@@ -346,10 +343,12 @@ const onNodeContentClick = async (
 }

 const extraMenuItems = computed(() => {
-  const contextMenuItems = menuTargetNode.value?.contextMenuItems
+  const targetNode = menuTargetNode.value
+  if (!targetNode) return []
+  const contextMenuItems = targetNode.contextMenuItems
  if (!contextMenuItems) return []
  return typeof contextMenuItems === 'function'
-    ? contextMenuItems(menuTargetNode.value!)
+    ? contextMenuItems(targetNode)
    : contextMenuItems
 })

--- a/src/utils/virtualListUtils.test.ts
+++ b/src/utils/virtualListUtils.test.ts
@@ -5,7 +5,9 @@ import type { RenderedTreeExplorerNode } from '@/types/treeExplorerTypes'
 import type { WindowRange } from './virtualListUtils'
 import {
  applyWindow,
+  calculateChildrenListBounds,
  calculateSpacerHeightsVariable,
+  calculateTreePositionsAndHeights,
  calculateWindowRangeByHeights,
  createInitialWindowRange,
  mergeWindowRange
@@ -236,4 +238,324 @@ describe('virtualListUtils', () => {
      expect(range.start).toBeLessThanOrEqual(20)
    })
  })
+
+  describe('calculateTreePositionsAndHeights', () => {
+    type TestNode = { key: string; children?: TestNode[] }
+
+    it('calculates positions and heights for flat tree', () => {
+      const root: { children?: TestNode[] } = {
+        children: [{ key: 'a' }, { key: 'b' }, { key: 'c' }]
+      }
+
+      const { positions, heights } = calculateTreePositionsAndHeights({
+        root,
+        itemHeight: 32,
+        getChildren: (node) => node.children,
+        isExpanded: () => false
+      })
+
+      expect(positions.get('a')).toBe(0)
+      expect(positions.get('b')).toBe(32)
+      expect(positions.get('c')).toBe(64)
+
+      expect(heights.get('a')).toBe(32)
+      expect(heights.get('b')).toBe(32)
+      expect(heights.get('c')).toBe(32)
+    })
+
+    it('calculates positions and heights for nested tree with expanded nodes', () => {
+      const root: { children?: TestNode[] } = {
+        children: [
+          {
+            key: 'parent',
+            children: [{ key: 'child1' }, { key: 'child2' }]
+          }
+        ]
+      }
+
+      const { positions, heights } = calculateTreePositionsAndHeights({
+        root,
+        itemHeight: 32,
+        getChildren: (node) => node.children,
+        isExpanded: (node) => node.key === 'parent'
+      })
+
+      expect(positions.get('parent')).toBe(0)
+      expect(positions.get('child1')).toBe(32)
+      expect(positions.get('child2')).toBe(64)
+
+      expect(heights.get('parent')).toBe(96) // 32 + 32 + 32
+      expect(heights.get('child1')).toBe(32)
+      expect(heights.get('child2')).toBe(32)
+    })
+
+    it('calculates positions and heights for nested tree with collapsed nodes', () => {
+      const root: { children?: TestNode[] } = {
+        children: [
+          {
+            key: 'parent',
+            children: [{ key: 'child1' }, { key: 'child2' }]
+          }
+        ]
+      }
+
+      const { positions, heights } = calculateTreePositionsAndHeights({
+        root,
+        itemHeight: 32,
+        getChildren: (node) => node.children,
+        isExpanded: () => false
+      })
+
+      expect(positions.get('parent')).toBe(0)
+      expect(heights.get('parent')).toBe(32) // Only parent height, children not included
+    })
+
+    it('handles deeply nested tree', () => {
+      const root: { children?: TestNode[] } = {
+        children: [
+          {
+            key: 'level1',
+            children: [
+              {
+                key: 'level2',
+                children: [{ key: 'level3' }]
+              }
+            ]
+          }
+        ]
+      }
+
+      const { positions, heights } = calculateTreePositionsAndHeights({
+        root,
+        itemHeight: 32,
+        getChildren: (node) => node.children,
+        isExpanded: () => true
+      })
+
+      expect(positions.get('level1')).toBe(0)
+      expect(positions.get('level2')).toBe(32)
+      expect(positions.get('level3')).toBe(64)
+
+      expect(heights.get('level1')).toBe(96) // 32 + 32 + 32
+      expect(heights.get('level2')).toBe(64) // 32 + 32
+      expect(heights.get('level3')).toBe(32)
+    })
+
+    it('handles empty tree', () => {
+      const root: { children?: TestNode[] } = { children: [] }
+
+      const { positions, heights } = calculateTreePositionsAndHeights({
+        root,
+        itemHeight: 32,
+        getChildren: (node) => node.children,
+        isExpanded: () => false
+      })
+
+      expect(positions.size).toBe(0)
+      expect(heights.size).toBe(0)
+    })
+
+    it('handles root without children', () => {
+      const root: { children?: TestNode[] } = {}
+
+      const { positions, heights } = calculateTreePositionsAndHeights({
+        root,
+        itemHeight: 32,
+        getChildren: (node) => node.children,
+        isExpanded: () => false
+      })
+
+      expect(positions.size).toBe(0)
+      expect(heights.size).toBe(0)
+    })
+
+    it('handles mixed expanded and collapsed nodes', () => {
+      const root: { children?: TestNode[] } = {
+        children: [
+          {
+            key: 'expanded',
+            children: [{ key: 'child1' }, { key: 'child2' }]
+          },
+          {
+            key: 'collapsed',
+            children: [{ key: 'child3' }]
+          }
+        ]
+      }
+
+      const { positions, heights } = calculateTreePositionsAndHeights({
+        root,
+        itemHeight: 32,
+        getChildren: (node) => node.children,
+        isExpanded: (node) => node.key === 'expanded'
+      })
+
+      expect(positions.get('expanded')).toBe(0)
+      expect(positions.get('child1')).toBe(32)
+      expect(positions.get('child2')).toBe(64)
+      expect(positions.get('collapsed')).toBe(96)
+
+      expect(heights.get('expanded')).toBe(96) // 32 + 32 + 32
+      expect(heights.get('collapsed')).toBe(32) // Only collapsed node height
+    })
+  })
+
+  describe('calculateChildrenListBounds', () => {
+    type TestNode = { key: string }
+
+    it('calculates bounds for node with children', () => {
+      const node: TestNode = { key: 'parent' }
+      const children: TestNode[] = [
+        { key: 'child1' },
+        { key: 'child2' },
+        { key: 'child3' }
+      ]
+
+      const nodePositions = new Map<string, number>([
+        ['parent', 0],
+        ['child1', 32],
+        ['child2', 64],
+        ['child3', 96]
+      ])
+
+      const nodeHeights = new Map<string, number>([
+        ['parent', 128],
+        ['child1', 32],
+        ['child2', 32],
+        ['child3', 32]
+      ])
+
+      const result = calculateChildrenListBounds({
+        node,
+        children,
+        nodePositions,
+        nodeHeights,
+        itemHeight: 32
+      })
+
+      expect(result.listStart).toBe(32) // parent position (0) + itemHeight (32)
+      expect(result.listEnd).toBe(128) // child3 position (96) + child3 height (32)
+    })
+
+    it('handles empty children array', () => {
+      const node: TestNode = { key: 'parent' }
+      const children: TestNode[] = []
+
+      const nodePositions = new Map<string, number>([['parent', 100]])
+      const nodeHeights = new Map<string, number>([['parent', 32]])
+
+      const result = calculateChildrenListBounds({
+        node,
+        children,
+        nodePositions,
+        nodeHeights,
+        itemHeight: 32
+      })
+
+      expect(result.listStart).toBe(132) // parent position (100) + itemHeight (32)
+      expect(result.listEnd).toBe(132) // Same as listStart when no children
+    })
+
+    it('uses default values when node position is missing', () => {
+      const node: TestNode = { key: 'parent' }
+      const children: TestNode[] = [{ key: 'child1' }]
+
+      const nodePositions = new Map<string, number>()
+      const nodeHeights = new Map<string, number>([['child1', 32]])
+
+      const result = calculateChildrenListBounds({
+        node,
+        children,
+        nodePositions,
+        nodeHeights,
+        itemHeight: 32
+      })
+
+      expect(result.listStart).toBe(32) // default node position (0) + itemHeight (32)
+      expect(result.listEnd).toBe(64) // default child position (32) + child height (32)
+    })
+
+    it('uses default values when child position or height is missing', () => {
+      const node: TestNode = { key: 'parent' }
+      const children: TestNode[] = [{ key: 'child1' }]
+
+      const nodePositions = new Map<string, number>([
+        ['parent', 0]
+        // child1 position missing
+      ])
+      const nodeHeights = new Map<string, number>()
+      // child1 height missing
+
+      const result = calculateChildrenListBounds({
+        node,
+        children,
+        nodePositions,
+        nodeHeights,
+        itemHeight: 32
+      })
+
+      expect(result.listStart).toBe(32) // parent position (0) + itemHeight (32)
+      expect(result.listEnd).toBe(64) // default child position (32) + default height (32)
+    })
+
+    it('handles single child', () => {
+      const node: TestNode = { key: 'parent' }
+      const children: TestNode[] = [{ key: 'child1' }]
+
+      const nodePositions = new Map<string, number>([
+        ['parent', 0],
+        ['child1', 32]
+      ])
+
+      const nodeHeights = new Map<string, number>([
+        ['parent', 64],
+        ['child1', 32]
+      ])
+
+      const result = calculateChildrenListBounds({
+        node,
+        children,
+        nodePositions,
+        nodeHeights,
+        itemHeight: 32
+      })
+
+      expect(result.listStart).toBe(32)
+      expect(result.listEnd).toBe(64)
+    })
+
+    it('handles children with variable heights', () => {
+      const node: TestNode = { key: 'parent' }
+      const children: TestNode[] = [
+        { key: 'child1' },
+        { key: 'child2' },
+        { key: 'child3' }
+      ]
+
+      const nodePositions = new Map<string, number>([
+        ['parent', 0],
+        ['child1', 32],
+        ['child2', 64],
+        ['child3', 96]
+      ])
+
+      const nodeHeights = new Map<string, number>([
+        ['parent', 160],
+        ['child1', 32],
+        ['child2', 64], // Larger height
+        ['child3', 32]
+      ])
+
+      const result = calculateChildrenListBounds({
+        node,
+        children,
+        nodePositions,
+        nodeHeights,
+        itemHeight: 32
+      })
+
+      expect(result.listStart).toBe(32)
+      expect(result.listEnd).toBe(128) // child3 position (96) + child3 height (32)
+    })
+  })
 })
--- a/src/utils/virtualListUtils.ts
+++ b/src/utils/virtualListUtils.ts
@@ -168,3 +168,81 @@ export function createInitialWindowRange(
    end: Math.min(windowSize, totalChildren)
  }
 }
+
+/**
+ * Calculate positions and heights for tree nodes in a single traversal
+ * @param root - Root node with children
+ * @param options - Configuration options
+ * @returns Maps of node positions and heights
+ */
+export function calculateTreePositionsAndHeights<T extends { key: string }>({
+  root,
+  itemHeight,
+  getChildren,
+  isExpanded
+}: {
+  root: { children?: T[] }
+  itemHeight: number
+  getChildren: (node: T) => T[] | undefined
+  isExpanded: (node: T) => boolean
+}): { positions: Map<string, number>; heights: Map<string, number> } {
+  const nodePositions = new Map<string, number>()
+  const nodeHeights = new Map<string, number>()
+  let currentPos = 0
+
+  const traverse = (node: T): number => {
+    nodePositions.set(node.key, currentPos)
+    currentPos += itemHeight
+
+    let nodeHeight = itemHeight
+    const children = getChildren(node)
+    if (isExpanded(node) && children) {
+      for (const child of children) {
+        nodeHeight += traverse(child)
+      }
+    }
+
+    nodeHeights.set(node.key, nodeHeight)
+    return nodeHeight
+  }
+
+  root.children?.forEach(traverse)
+  return { positions: nodePositions, heights: nodeHeights }
+}
+
+/**
+ * Calculate list start and end positions for a parent node's children
+ * @param node - Parent node
+ * @param children - Children array
+ * @param nodePositions - Map of node positions
+ * @param nodeHeights - Map of node heights
+ * @param itemHeight - Height of a single item
+ * @returns Object with listStart and listEnd positions
+ */
+export function calculateChildrenListBounds<T extends { key: string }>({
+  node,
+  children,
+  nodePositions,
+  nodeHeights,
+  itemHeight
+}: {
+  node: T
+  children: T[]
+  nodePositions: Map<string, number>
+  nodeHeights: Map<string, number>
+  itemHeight: number
+}): { listStart: number; listEnd: number } {
+  const nodeStart = nodePositions.get(node.key) ?? 0
+  const listStart = nodeStart + itemHeight
+
+  if (children.length === 0) {
+    return { listStart, listEnd: listStart }
+  }
+
+  const lastChild = children.at(-1)!
+  const lastChildStart = nodePositions.get(lastChild.key) ?? listStart
+  const lastChildHeight = nodeHeights.get(lastChild.key) ?? itemHeight
+  const listEnd = lastChildStart + lastChildHeight
+
+  return { listStart, listEnd }
+}