perf(admin-ui): Improve performance of Collection list view

Fixes #1123. This commit eliminates an expensive function that was being called quadratically with the number of collections, on each interaction.
vendure-ecommerce · Oct 28, 2021 · 4bf6dff · 4bf6dff
1 parent 74250ea
commit 4bf6dff
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 20 deletions.
diff --git a/...min-ui/src/lib/catalog/src/components/collection-tree/collection-tree-node.component.html b/...min-ui/src/lib/catalog/src/components/collection-tree/collection-tree-node.component.html
@@ -51,7 +51,7 @@
                 <clr-icon shape="drag-handle" size="24"></clr-icon>
             </div>
             <vdr-dropdown>
-                <button class="icon-button" vdrDropdownTrigger>
+                <button class="icon-button" vdrDropdownTrigger (click)="getMoveListItems(collection)">
                     <clr-icon shape="ellipsis-vertical"></clr-icon>
                 </button>
                 <vdr-dropdown-menu vdrPosition="bottom-right">
@@ -88,7 +88,7 @@ <h4 class="dropdown-header">{{ 'catalog.move-to' | translate }}</h4>
                     <button
                         type="button"
                         vdrDropdownItem
-                        *ngFor="let item of getMoveListItems(collection)"
+                        *ngFor="let item of moveListItems"
                         (click)="move(collection, item.id)"
                         [disabled]="!(hasUpdatePermission$ | async)"
                     >

diff --git a/...admin-ui/src/lib/catalog/src/components/collection-tree/collection-tree-node.component.ts b/...admin-ui/src/lib/catalog/src/components/collection-tree/collection-tree-node.component.ts
@@ -30,6 +30,7 @@ export class CollectionTreeNodeComponent implements OnInit, OnChanges {
     @Input() expandAll = false;
     hasUpdatePermission$: Observable<boolean>;
     hasDeletePermission$: Observable<boolean>;
+    moveListItems: Array<{ path: string; id: string }> = [];
 
     constructor(
         @SkipSelf() @Optional() private parent: CollectionTreeNodeComponent,
@@ -74,23 +75,8 @@ export class CollectionTreeNodeComponent implements OnInit, OnChanges {
         return item.id;
     }
 
-    getMoveListItems(collection: CollectionPartial): Array<{ path: string; id: string }> {
-        const visit = (
-            node: TreeNode<any>,
-            parentPath: string[],
-            output: Array<{ path: string; id: string }>,
-        ) => {
-            if (node.id !== collection.id) {
-                const path = parentPath.concat(node.name);
-                const parentId = collection.parent && collection.parent.id;
-                if (node.id !== parentId) {
-                    output.push({ path: path.slice(1).join(' / ') || 'root', id: node.id });
-                }
-                node.children.forEach(child => visit(child, path, output));
-            }
-            return output;
-        };
-        return visit(this.root.collectionTree, [], []);
+    getMoveListItems(collection: CollectionPartial) {
+        this.moveListItems = this.root.getMoveListItems(collection);
     }
 
     move(collection: CollectionPartial, parentId: string) {

diff --git a/...ages/admin-ui/src/lib/catalog/src/components/collection-tree/collection-tree.component.ts b/...ages/admin-ui/src/lib/catalog/src/components/collection-tree/collection-tree.component.ts
@@ -10,7 +10,7 @@ import {
 } from '@angular/core';
 import { Collection } from '@vendure/admin-ui/core';
 
-import { arrayToTree, HasParent, RootNode } from './array-to-tree';
+import { arrayToTree, HasParent, RootNode, TreeNode } from './array-to-tree';
 
 export type RearrangeEvent = { collectionId: string; parentId: string; index: number };
 export type CollectionPartial = Pick<Collection.Fragment, 'id' | 'parent' | 'name'>;
@@ -28,10 +28,12 @@ export class CollectionTreeComponent implements OnChanges {
     @Output() rearrange = new EventEmitter<RearrangeEvent>();
     @Output() deleteCollection = new EventEmitter<string>();
     collectionTree: RootNode<CollectionPartial>;
+    private allMoveListItems: Array<{ path: string; id: string; ancestorIdPath: Set<string> }> = [];
 
     ngOnChanges(changes: SimpleChanges) {
         if ('collections' in changes && this.collections) {
             this.collectionTree = arrayToTree(this.collections, this.collectionTree);
+            this.allMoveListItems = [];
         }
     }
 
@@ -57,6 +59,35 @@ export class CollectionTreeComponent implements OnChanges {
         this.deleteCollection.emit(id);
     }
 
+    getMoveListItems(collection: CollectionPartial) {
+        if (this.allMoveListItems.length === 0) {
+            this.allMoveListItems = this.calculateAllMoveListItems();
+        }
+        return this.allMoveListItems.filter(
+            item =>
+                item.id !== collection.id &&
+                !item.ancestorIdPath.has(collection.id) &&
+                item.id !== collection.parent?.id,
+        );
+    }
+
+    calculateAllMoveListItems() {
+        const visit = (
+            node: TreeNode<any>,
+            parentPath: string[],
+            ancestorIdPath: Set<string>,
+            output: Array<{ path: string; id: string; ancestorIdPath: Set<string> }>,
+        ) => {
+            const path = parentPath.concat(node.name);
+            output.push({ path: path.slice(1).join(' / ') || 'root', id: node.id, ancestorIdPath });
+            node.children.forEach(child =>
+                visit(child, path, new Set<string>([...ancestorIdPath, node.id]), output),
+            );
+            return output;
+        };
+        return visit(this.collectionTree, [], new Set<string>(), []);
+    }
+
     private isRootNode<T extends HasParent>(node: T | RootNode<T>): node is RootNode<T> {
         return !node.hasOwnProperty('parent');
     }