Fix loss of tree node state when switching views

This commit fixes an issue where the check state of categories was lost
when toggling between card and tree views. This is solved by immediately
emitting node state changes for all nodes. This ensures consistent view
transitions without any loss of node state information.

Furthermore, this commit includes added unit tests for the modified code
sections.

src/presentation/components/Scripts/View/Tree/TreeView/Bindings/TreeNodeStateChangedEmittedEvent.ts

@@ -1,7 +1,8 @@
-import { NodeStateChangedEvent } from '../Node/State/StateAccess';
+import { TreeNodeStateDescriptor } from '../Node/State/StateDescriptor';
 import { ReadOnlyTreeNode } from '../Node/TreeNode';
 
 export interface TreeNodeStateChangedEmittedEvent {
-  readonly change: NodeStateChangedEvent;
   readonly node: ReadOnlyTreeNode;
+  readonly oldState?: TreeNodeStateDescriptor;
+  readonly newState: TreeNodeStateDescriptor;
 }

src/presentation/components/Scripts/View/Tree/TreeView/Rendering/DelayScheduler.ts

@@ -0,0 +1,3 @@
+export interface DelayScheduler {
+  scheduleNext(callback: () => void, delayInMs: number): void;
+}

src/presentation/components/Scripts/View/Tree/TreeView/Rendering/TimeoutDelayScheduler.ts

@@ -0,0 +1,28 @@
+import { DelayScheduler } from './DelayScheduler';
+
+export class TimeoutDelayScheduler implements DelayScheduler {
+  private timeoutId: ReturnType<typeof setTimeout> | undefined = undefined;
+
+  constructor(private readonly timer: TimeFunctions = {
+    clearTimeout: globalThis.clearTimeout.bind(globalThis),
+    setTimeout: globalThis.setTimeout.bind(globalThis),
+  }) { }
+
+  public scheduleNext(callback: () => void, delayInMs: number): void {
+    this.clear();
+    this.timeoutId = this.timer.setTimeout(callback, delayInMs);
+  }
+
+  private clear(): void {
+    if (this.timeoutId === undefined) {
+      return;
+    }
+    this.timer.clearTimeout(this.timeoutId);
+    this.timeoutId = undefined;
+  }
+}
+
+export interface TimeFunctions {
+  clearTimeout(id: ReturnType<typeof setTimeout>): void;
+  setTimeout(callback: () => void, delayInMs: number): ReturnType<typeof setTimeout>;
+}

src/presentation/components/Scripts/View/Tree/TreeView/Rendering/UseGradualNodeRendering.ts

@@ -6,39 +6,37 @@ import { useNodeStateChangeAggregator } from '../UseNodeStateChangeAggregator';
 import { TreeRoot } from '../TreeRoot/TreeRoot';
 import { useCurrentTreeNodes } from '../UseCurrentTreeNodes';
 import { NodeRenderingStrategy } from './NodeRenderingStrategy';
+import { DelayScheduler } from './DelayScheduler';
+import { TimeoutDelayScheduler } from './TimeoutDelayScheduler';
 
 /**
  * Renders tree nodes gradually to prevent UI freeze when loading large amounts of nodes.
  */
 export function useGradualNodeRendering(
   treeWatcher: WatchSource<TreeRoot>,
+  useChangeAggregator = useNodeStateChangeAggregator,
+  useTreeNodes = useCurrentTreeNodes,
+  scheduler: DelayScheduler = new TimeoutDelayScheduler(),
+  initialBatchSize = 30,
+  subsequentBatchSize = 5,
 ): NodeRenderingStrategy {
   const nodesToRender = new Set<ReadOnlyTreeNode>();
   const nodesBeingRendered = shallowRef(new Set<ReadOnlyTreeNode>());
-  let isFirstRender = true;
   let isRenderingInProgress = false;
   const renderingDelayInMs = 50;
-  const initialBatchSize = 30;
-  const subsequentBatchSize = 5;
 
-  const { onNodeStateChange } = useNodeStateChangeAggregator(treeWatcher);
-  const { nodes } = useCurrentTreeNodes(treeWatcher);
+  const { onNodeStateChange } = useChangeAggregator(treeWatcher);
+  const { nodes } = useTreeNodes(treeWatcher);
 
   const orderedNodes = computed<readonly ReadOnlyTreeNode[]>(() => nodes.value.flattenedNodes);
 
-  watch(() => orderedNodes.value, (newNodes) => {
-    newNodes.forEach((node) => updateNodeRenderQueue(node));
-  }, { immediate: true });
-
-  function updateNodeRenderQueue(node: ReadOnlyTreeNode) {
-    if (node.state.current.isVisible
+  function updateNodeRenderQueue(node: ReadOnlyTreeNode, isVisible: boolean) {
+    if (isVisible
         && !nodesToRender.has(node)
         && !nodesBeingRendered.value.has(node)) {
       nodesToRender.add(node);
-      if (!isRenderingInProgress) {
-        scheduleRendering();
-      }
-    } else if (!node.state.current.isVisible) {
+      beginRendering();
+    } else if (!isVisible) {
       if (nodesToRender.has(node)) {
         nodesToRender.delete(node);
       }
@@ -49,47 +47,58 @@ export function useGradualNodeRendering(
     }
   }
 
-  onNodeStateChange((node, change) => {
-    if (change.newState.isVisible === change.oldState.isVisible) {
+  watch(() => orderedNodes.value, (newNodes) => {
+    nodesToRender.clear();
+    nodesBeingRendered.value.clear();
+    if (!newNodes?.length) {
+      triggerRef(nodesBeingRendered);
       return;
     }
-    updateNodeRenderQueue(node);
+    newNodes
+      .filter((node) => node.state.current.isVisible)
+      .forEach((node) => nodesToRender.add(node));
+    beginRendering();
+  }, { immediate: true });
+
+  onNodeStateChange((change) => {
+    if (change.newState.isVisible === change.oldState?.isVisible) {
+      return;
+    }
+    updateNodeRenderQueue(change.node, change.newState.isVisible);
   });
 
-  scheduleRendering();
-
-  function scheduleRendering() {
-    if (isFirstRender) {
-      renderNodeBatch();
-      isFirstRender = false;
-    } else {
-      const delayScheduler = new DelayScheduler(renderingDelayInMs);
-      delayScheduler.schedule(renderNodeBatch);
+  function beginRendering() {
+    if (isRenderingInProgress) {
+      return;
     }
+    renderNextBatch(initialBatchSize);
   }
 
-  function renderNodeBatch() {
+  function renderNextBatch(batchSize: number) {
     if (nodesToRender.size === 0) {
       isRenderingInProgress = false;
       return;
     }
     isRenderingInProgress = true;
-    const batchSize = isFirstRender ? initialBatchSize : subsequentBatchSize;
     const sortedNodes = Array.from(nodesToRender).sort(
       (a, b) => orderedNodes.value.indexOf(a) - orderedNodes.value.indexOf(b),
     );
     const currentBatch = sortedNodes.slice(0, batchSize);
+    if (currentBatch.length === 0) {
+      return;
+    }
     currentBatch.forEach((node) => {
       nodesToRender.delete(node);
       nodesBeingRendered.value.add(node);
     });
     triggerRef(nodesBeingRendered);
-    if (nodesToRender.size > 0) {
-      scheduleRendering();
-    }
+    scheduler.scheduleNext(
+      () => renderNextBatch(subsequentBatchSize),
+      renderingDelayInMs,
+    );
   }
 
-  function shouldNodeBeRendered(node: ReadOnlyTreeNode) {
+  function shouldNodeBeRendered(node: ReadOnlyTreeNode): boolean {
     return nodesBeingRendered.value.has(node);
   }
 
@@ -97,21 +106,3 @@ export function useGradualNodeRendering(
     shouldRender: shouldNodeBeRendered,
   };
 }
-
-class DelayScheduler {
-  private timeoutId: ReturnType<typeof setTimeout> = null;
-
-  constructor(private delay: number) {}
-
-  schedule(callback: () => void) {
-    this.clear();
-    this.timeoutId = setTimeout(callback, this.delay);
-  }
-
-  clear() {
-    if (this.timeoutId) {
-      clearTimeout(this.timeoutId);
-      this.timeoutId = null;
-    }
-  }
-}

src/presentation/components/Scripts/View/Tree/TreeView/TreeView.vue

@@ -68,8 +68,13 @@ export default defineComponent({
     const nodeRenderingScheduler = useGradualNodeRendering(() => tree);
 
     const { onNodeStateChange } = useNodeStateChangeAggregator(() => tree);
-    onNodeStateChange((node, change) => {
-      emit('nodeStateChanged', { node, change });
+
+    onNodeStateChange((change) => {
+      emit('nodeStateChanged', {
+        node: change.node,
+        newState: change.newState,
+        oldState: change.oldState,
+      });
     });
 
     onMounted(() => {

src/presentation/components/Scripts/View/Tree/TreeView/UseAutoUpdateChildrenCheckState.ts

@@ -6,14 +6,15 @@ import { TreeNodeCheckState } from './Node/State/CheckState';
 
 export function useAutoUpdateChildrenCheckState(
   treeWatcher: WatchSource<TreeRoot>,
+  useChangeAggregator = useNodeStateChangeAggregator,
 ) {
-  const { onNodeStateChange } = useNodeStateChangeAggregator(treeWatcher);
+  const { onNodeStateChange } = useChangeAggregator(treeWatcher);
 
-  onNodeStateChange((node, change) => {
-    if (change.newState.checkState === change.oldState.checkState) {
+  onNodeStateChange((change) => {
+    if (change.newState.checkState === change.oldState?.checkState) {
       return;
     }
-    updateChildrenCheckedState(node.hierarchy, change.newState.checkState);
+    updateChildrenCheckedState(change.node.hierarchy, change.newState.checkState);
   });
 }
 

src/presentation/components/Scripts/View/Tree/TreeView/UseAutoUpdateParentCheckState.ts

@@ -7,14 +7,15 @@ import { ReadOnlyTreeNode } from './Node/TreeNode';
 
 export function useAutoUpdateParentCheckState(
   treeWatcher: WatchSource<TreeRoot>,
+  useChangeAggregator = useNodeStateChangeAggregator,
 ) {
-  const { onNodeStateChange } = useNodeStateChangeAggregator(treeWatcher);
+  const { onNodeStateChange } = useChangeAggregator(treeWatcher);
 
-  onNodeStateChange((node, change) => {
-    if (change.newState.checkState === change.oldState.checkState) {
+  onNodeStateChange((change) => {
+    if (change.newState.checkState === change.oldState?.checkState) {
       return;
     }
-    updateNodeParentCheckedState(node.hierarchy);
+    updateNodeParentCheckedState(change.node.hierarchy);
   });
 }
 

src/presentation/components/Scripts/View/Tree/TreeView/UseCurrentTreeNodes.ts

@@ -8,7 +8,7 @@ import { QueryableNodes } from './TreeRoot/NodeCollection/Query/QueryableNodes';
 export function useCurrentTreeNodes(treeWatcher: WatchSource<TreeRoot>) {
   const { events } = inject(InjectionKeys.useAutoUnsubscribedEvents)();
 
-  const tree = ref<TreeRoot>();
+  const tree = ref<TreeRoot | undefined>();
   const nodes = ref<QueryableNodes | undefined>();
 
   watch(treeWatcher, (newTree) => {

src/presentation/components/Scripts/View/Tree/TreeView/UseNodeStateChangeAggregator.ts

@@ -1,35 +1,83 @@
-import { WatchSource, inject, watch } from 'vue';
+import {
+  WatchSource, inject, watch, ref,
+} from 'vue';
 import { InjectionKeys } from '@/presentation/injectionSymbols';
+import { IEventSubscription } from '@/infrastructure/Events/IEventSource';
 import { TreeRoot } from './TreeRoot/TreeRoot';
 import { TreeNode } from './Node/TreeNode';
 import { useCurrentTreeNodes } from './UseCurrentTreeNodes';
-import { NodeStateChangedEvent } from './Node/State/StateAccess';
+import { TreeNodeStateDescriptor } from './Node/State/StateDescriptor';
 
-type NodeStateChangeEventCallback = (
-  node: TreeNode,
-  stateChange: NodeStateChangedEvent,
-) => void;
+export type NodeStateChangeEventCallback = (args: NodeStateChangeEventArgs) => void;
 
-export function useNodeStateChangeAggregator(treeWatcher: WatchSource<TreeRoot>) {
-  const { nodes } = useCurrentTreeNodes(treeWatcher);
+export function useNodeStateChangeAggregator(
+  treeWatcher: WatchSource<TreeRoot>,
+  useTreeNodes = useCurrentTreeNodes,
+) {
+  const { nodes } = useTreeNodes(treeWatcher);
   const { events } = inject(InjectionKeys.useAutoUnsubscribedEvents)();
 
-  const onNodeChangeCallbacks = new Array<NodeStateChangeEventCallback>();
+  const onNodeChangeCallback = ref<NodeStateChangeEventCallback>();
 
-  watch(() => nodes.value, (newNodes) => {
-    events.unsubscribeAll();
-    newNodes.flattenedNodes.forEach((node) => {
-      events.register([
-        node.state.changed.on((stateChange) => {
-          onNodeChangeCallbacks.forEach((callback) => callback(node, stateChange));
-        }),
-      ]);
-    });
+  watch([
+    () => nodes.value,
+    () => onNodeChangeCallback.value,
+  ], ([newNodes, callback]) => {
+    if (!callback) { // might not be registered yet
+      return;
+    }
+    if (!newNodes || newNodes.flattenedNodes.length === 0) {
+      events.unsubscribeAll();
+      return;
+    }
+    const allNodes = newNodes.flattenedNodes;
+    events.unsubscribeAllAndRegister(
+      subscribeToNotifyOnFutureNodeChanges(allNodes, callback),
+    );
+    notifyCurrentNodeState(allNodes, callback);
   });
 
+  function onNodeStateChange(
+    callback: NodeStateChangeEventCallback,
+  ): void {
+    if (!callback) {
+      throw new Error('missing callback');
+    }
+    onNodeChangeCallback.value = callback;
+  }
+
   return {
-    onNodeStateChange: (
-      callback: NodeStateChangeEventCallback,
-    ) => onNodeChangeCallbacks.push(callback),
+    onNodeStateChange,
   };
 }
+
+export interface NodeStateChangeEventArgs {
+  readonly node: TreeNode;
+  readonly newState: TreeNodeStateDescriptor;
+  readonly oldState?: TreeNodeStateDescriptor;
+}
+
+function notifyCurrentNodeState(
+  nodes: readonly TreeNode[],
+  callback: NodeStateChangeEventCallback,
+) {
+  nodes.forEach((node) => {
+    callback({
+      node,
+      newState: node.state.current,
+    });
+  });
+}
+
+function subscribeToNotifyOnFutureNodeChanges(
+  nodes: readonly TreeNode[],
+  callback: NodeStateChangeEventCallback,
+): IEventSubscription[] {
+  return nodes.map((node) => node.state.changed.on((stateChange) => {
+    callback({
+      node,
+      oldState: stateChange.oldState,
+      newState: stateChange.newState,
+    });
+  }));
+}

src/presentation/components/Scripts/View/Tree/TreeViewAdapter/UseCollectionSelectionStateUpdater.ts

@@ -6,12 +6,12 @@ import { TreeNodeStateChangedEmittedEvent } from '../TreeView/Bindings/TreeNodeS
 export function useCollectionSelectionStateUpdater() {
   const { modifyCurrentState, currentState } = inject(InjectionKeys.useCollectionState)();
 
-  const updateNodeSelection = (event: TreeNodeStateChangedEmittedEvent) => {
-    const { node } = event;
+  function updateNodeSelection(change: TreeNodeStateChangedEmittedEvent) {
+    const { node } = change;
     if (node.hierarchy.isBranchNode) {
       return; // A category, let TreeView handle this
     }
-    if (event.change.oldState.checkState === event.change.newState.checkState) {
+    if (change.oldState?.checkState === change.newState.checkState) {
       return;
     }
     if (node.state.current.checkState === TreeNodeCheckState.Checked) {
@@ -30,7 +30,7 @@ export function useCollectionSelectionStateUpdater() {
         state.selection.removeSelectedScript(node.id);
       });
     }
-  };
+  }
 
   return {
     updateNodeSelection,