From 0a5baa6256a9462331427ab12cc9b65d819e39c1 Mon Sep 17 00:00:00 2001 From: Yourz Date: Thu, 22 Jan 2026 18:51:16 +0800 Subject: [PATCH] fix: update for coderabbit --- src/components/common/TreeExplorer.vue | 105 ++++---- src/utils/virtualListUtils.test.ts | 322 +++++++++++++++++++++++++ src/utils/virtualListUtils.ts | 78 ++++++ 3 files changed, 452 insertions(+), 53 deletions(-) diff --git a/src/components/common/TreeExplorer.vue b/src/components/common/TreeExplorer.vue index 4a4050465..0ac266202 100644 --- 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>('expandedKeys', { }) provide(InjectKeyExpandedKeys, expandedKeys) const selectionKeys = defineModel>('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 | null>(null) const menuTargetNode = ref(null) const renameEditingNode = ref(null) const parentNodeWindowRanges = ref>({}) +const nodeHeightsCache = ref>(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 => { - const nodePositions = new Map() - 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; heights: Map } => { + return calculateTreePositionsAndHeights({ + root, + itemHeight: DEFAULT_NODE_HEIGHT, + getChildren: (node) => node.children, + isExpanded: isNodeExpanded + }) } const calculateNodeWindowRange = ( node: RenderedTreeExplorerNode, nodePositions: Map, + nodeHeights: Map, 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 = {} 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(() => { @@ -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 }) diff --git a/src/utils/virtualListUtils.test.ts b/src/utils/virtualListUtils.test.ts index ef3af7189..4b597dcb4 100644 --- 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([ + ['parent', 0], + ['child1', 32], + ['child2', 64], + ['child3', 96] + ]) + + const nodeHeights = new Map([ + ['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([['parent', 100]]) + const nodeHeights = new Map([['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() + const nodeHeights = new Map([['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([ + ['parent', 0] + // child1 position missing + ]) + const nodeHeights = new Map() + // 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([ + ['parent', 0], + ['child1', 32] + ]) + + const nodeHeights = new Map([ + ['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([ + ['parent', 0], + ['child1', 32], + ['child2', 64], + ['child3', 96] + ]) + + const nodeHeights = new Map([ + ['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) + }) + }) }) diff --git a/src/utils/virtualListUtils.ts b/src/utils/virtualListUtils.ts index ee0057a98..b131056e5 100644 --- 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({ + root, + itemHeight, + getChildren, + isExpanded +}: { + root: { children?: T[] } + itemHeight: number + getChildren: (node: T) => T[] | undefined + isExpanded: (node: T) => boolean +}): { positions: Map; heights: Map } { + const nodePositions = new Map() + const nodeHeights = new Map() + 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({ + node, + children, + nodePositions, + nodeHeights, + itemHeight +}: { + node: T + children: T[] + nodePositions: Map + nodeHeights: Map + 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 } +}