From 1f266c33535f72b69c65985bf2eff27cd2c5a104 Mon Sep 17 00:00:00 2001
From: undergroundwires <undergroundwires@users.noreply.github.com>
Date: Sun, 6 Sep 2020 15:26:19 +0100
Subject: [PATCH] fix indeterminate state being lost

---
 .../SelectableTree/LiquorTree/LiquorTree.d.ts |   1 +
 .../LiquorTree/LiquorTreeOptions.ts           |   3 +-
 .../NodeWrapper/NodeStateUpdater.ts           |  35 ++-
 .../LiquorTree/NodeWrapper/NodeTranslator.ts  |   1 +
 .../SelectableTree/SelectableTree.vue         |  22 +-
 .../NodeWrapper/NodeStateUpdater.spec.ts      | 264 ++++++++++++------
 .../NodeWrapper/NodeTranslator.spec.ts        |   1 +
 7 files changed, 227 insertions(+), 100 deletions(-)

diff --git a/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/LiquorTree.d.ts b/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/LiquorTree.d.ts
index 4af1ff73..b3ed101c 100644
--- a/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/LiquorTree.d.ts
+++ b/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/LiquorTree.d.ts
@@ -20,6 +20,7 @@ declare module 'liquor-tree' {
     // https://github.com/amsik/liquor-tree/blob/master/src/lib/Node.js
     export interface ILiquorTreeNodeState {
         checked: boolean;
+        indeterminate: boolean;
     }
 
     export interface ILiquorTreeNode {
diff --git a/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/LiquorTreeOptions.ts b/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/LiquorTreeOptions.ts
index e7cff70f..9b6e4a2d 100644
--- a/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/LiquorTreeOptions.ts
+++ b/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/LiquorTreeOptions.ts
@@ -5,9 +5,10 @@ export class LiquorTreeOptions implements ILiquorTreeOptions {
     public readonly checkbox = true;
     public readonly checkOnSelect = true;
     /*  For checkbox mode only. Children will have the same checked state as their parent.
+        ⚠️ Setting this false, does not update indeterminate state of nodes.
         This is false as it's handled manually to be able to batch select for performance + highlighting */
     public readonly autoCheckChildren = false;
-    public readonly parentSelect = false;
+    public readonly parentSelect = true;
     public readonly keyboardNavigation = true;
     public readonly filter = { // Wrap this in an arrow function as setting filter directly does not work JS APIs
         emptyText: this.liquorTreeFilter.emptyText,
diff --git a/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater.ts b/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater.ts
index 1e2a1c9f..d859b879 100644
--- a/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater.ts
+++ b/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater.ts
@@ -1,14 +1,37 @@
-import { ILiquorTreeNode } from 'liquor-tree';
+import { ILiquorTreeNode, ILiquorTreeNodeState } from 'liquor-tree';
 import { NodeType } from './../../Node/INode';
 
-export function getNewCheckedState(
-    oldNode: ILiquorTreeNode,
+export function getNewState(
+    node: ILiquorTreeNode,
+    selectedNodeIds: ReadonlyArray<string>): ILiquorTreeNodeState {
+    const checked = getNewCheckedState(node, selectedNodeIds);
+    const indeterminate = !checked && getNewIndeterminateState(node, selectedNodeIds);
+    return {
+        checked, indeterminate,
+    };
+}
+
+function getNewIndeterminateState(
+    node: ILiquorTreeNode,
     selectedNodeIds: ReadonlyArray<string>): boolean {
-    switch (oldNode.data.type) {
+    switch (node.data.type) {
         case NodeType.Script:
-            return selectedNodeIds.some((id) => id === oldNode.id);
+            return false;
         case NodeType.Category:
-            return parseAllSubScriptIds(oldNode).every((id) => selectedNodeIds.includes(id));
+            return parseAllSubScriptIds(node).some((id) => selectedNodeIds.includes(id));
+        default:
+            throw new Error('Unknown node type');
+    }
+}
+
+function getNewCheckedState(
+    node: ILiquorTreeNode,
+    selectedNodeIds: ReadonlyArray<string>): boolean {
+    switch (node.data.type) {
+        case NodeType.Script:
+            return selectedNodeIds.some((id) => id === node.id);
+        case NodeType.Category:
+            return parseAllSubScriptIds(node).every((id) => selectedNodeIds.includes(id));
         default:
             throw new Error('Unknown node type');
     }
diff --git a/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeTranslator.ts b/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeTranslator.ts
index 7e947c6d..f5c5bbe0 100644
--- a/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeTranslator.ts
+++ b/src/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeTranslator.ts
@@ -23,6 +23,7 @@ export function toNewLiquorTreeNode(node: INode): ILiquorTreeNewNode {
         text: node.text,
         state: {
             checked: false,
+            indeterminate: false,
         },
         children: convertChildren(node.children, toNewLiquorTreeNode),
         data: {
diff --git a/src/presentation/Scripts/ScriptsTree/SelectableTree/SelectableTree.vue b/src/presentation/Scripts/ScriptsTree/SelectableTree/SelectableTree.vue
index 078eb3b8..74d330a9 100644
--- a/src/presentation/Scripts/ScriptsTree/SelectableTree/SelectableTree.vue
+++ b/src/presentation/Scripts/ScriptsTree/SelectableTree/SelectableTree.vue
@@ -18,14 +18,15 @@
 
 <script lang="ts">
     import { Component, Prop, Vue, Watch } from 'vue-property-decorator';
-    import LiquorTree, { ILiquorTreeNewNode, ILiquorTreeExistingNode, ILiquorTree } from 'liquor-tree';
+    import LiquorTree from 'liquor-tree';
     import Node from './Node/Node.vue';
     import { INode } from './Node/INode';
     import { convertExistingToNode, toNewLiquorTreeNode } from './LiquorTree/NodeWrapper/NodeTranslator';
     import { INodeSelectedEvent } from './/INodeSelectedEvent';
-    import { getNewCheckedState } from './LiquorTree/NodeWrapper/NodeStateUpdater';
+    import { getNewState } from './LiquorTree/NodeWrapper/NodeStateUpdater';
     import { LiquorTreeOptions } from './LiquorTree/LiquorTreeOptions';
     import { FilterPredicate, NodePredicateFilter } from './LiquorTree/NodeWrapper/NodePredicateFilter';
+    import { ILiquorTreeNewNode, ILiquorTreeExistingNode, ILiquorTree, ILiquorTreeNode, ILiquorTreeNodeState } from 'liquor-tree';
 
     /** Wrapper for Liquor Tree, reveals only abstracted INode for communication */
     @Component({
@@ -49,7 +50,7 @@
                 const initialNodes = this.initialNodes.map((node) => toNewLiquorTreeNode(node));
                 if (this.selectedNodeIds) {
                     recurseDown(initialNodes,
-                        (node) => node.state.checked = getNewCheckedState(node, this.selectedNodeIds));
+                        (node) => node.state = updateState(node.state, node, this.selectedNodeIds));
                 }
                 this.initialLiquourTreeNodes = initialNodes;
             } else {
@@ -82,11 +83,11 @@
         @Watch('selectedNodeIds')
         public setSelectedStatusAsync(selectedNodeIds: ReadonlyArray<string>) {
             if (!selectedNodeIds) {
-                throw new Error('Selected nodes are undefined');
+                throw new Error('SelectedrecurseDown nodes are undefined');
             }
-            this.getLiquorTreeApi().recurseDown((node) => {
-                node.states.checked = getNewCheckedState(node, selectedNodeIds);
-            });
+            this.getLiquorTreeApi().recurseDown(
+                (node) => node.states = updateState(node.states, node, selectedNodeIds),
+            );
         }
 
         private getLiquorTreeApi(): ILiquorTree {
@@ -97,6 +98,13 @@
         }
     }
 
+    function updateState(
+        old: ILiquorTreeNodeState,
+        node: ILiquorTreeNode,
+        selectedNodeIds: ReadonlyArray<string>): ILiquorTreeNodeState {
+        return {...old, ...getNewState(node, selectedNodeIds)};
+    }
+
     function recurseDown(
         nodes: ReadonlyArray<ILiquorTreeNewNode>,
         handler: (node: ILiquorTreeNewNode) => void) {
diff --git a/tests/unit/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater.spec.ts b/tests/unit/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater.spec.ts
index 297fde47..8945f7e6 100644
--- a/tests/unit/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater.spec.ts
+++ b/tests/unit/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater.spec.ts
@@ -2,97 +2,189 @@ import 'mocha';
 import { expect } from 'chai';
 import { ILiquorTreeNode } from 'liquor-tree';
 import { NodeType } from '@/presentation/Scripts/ScriptsTree/SelectableTree/Node/INode';
-import { getNewCheckedState } from '@/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater';
+import { getNewState } from '@/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeStateUpdater';
 
-describe('getNewCheckedState', () => {
-    describe('script node', () => {
-        it('state is true when selected', () => {
-            // arrange
-            const node = getScriptNode();
-            const selectedScriptNodeIds = [ 'a', 'b', node.id, 'c' ];
-            // act
-            const actual = getNewCheckedState(node, selectedScriptNodeIds);
-            // assert
-            expect(actual).to.equal(true);
+describe('getNewState', () => {
+    describe('checked', () => {
+        describe('script node', () => {
+            it('true when selected', () => {
+                // arrange
+                const node = getScriptNode();
+                const selectedScriptNodeIds = [ 'a', 'b', node.id, 'c' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.checked).to.equal(true);
+            });
+            it('false when unselected', () => {
+                // arrange
+                const node = getScriptNode();
+                const selectedScriptNodeIds = [ 'a', 'b', 'c' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.checked).to.equal(false);
+            });
         });
-        it('state is false when unselected', () => {
-            // arrange
-            const node = getScriptNode();
-            const selectedScriptNodeIds = [ 'a', 'b', 'c' ];
-            // act
-            const actual = getNewCheckedState(node, selectedScriptNodeIds);
-            // assert
-            expect(actual).to.equal(false);
+        describe('category node', () => {
+            it('true when every child selected', () => {
+                // arrange
+                const node = {
+                    id: '1',
+                    data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                    children: [
+                        {   id: '2',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('a'), getScriptNode('b') ],
+                        },
+                        {   id: '3',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('c') ],
+                        },
+                    ],
+                };
+                const selectedScriptNodeIds = [ 'a', 'b', 'c' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.checked).to.equal(true);
+            });
+            it('false when none of the children is selected', () => {
+                // arrange
+                const node = {
+                    id: '1',
+                    data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                    children: [
+                        {   id: '2',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('a'), getScriptNode('b') ],
+                        },
+                        {   id: '3',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('c') ],
+                        },
+                    ],
+                };
+                const selectedScriptNodeIds = [ 'none', 'of', 'them', 'are', 'selected' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.checked).to.equal(false);
+            });
+            it('false when some of the children is selected', () => {
+                // arrange
+                const node = {
+                    id: '1',
+                    data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                    children: [
+                        {
+                            id: '2',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('a'), getScriptNode('b') ],
+                        },
+                        {
+                            id: '3',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('c') ],
+                        },
+                    ],
+                };
+                const selectedScriptNodeIds = [ 'a', 'c', 'unrelated' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.checked).to.equal(false);
+            });
         });
     });
-    describe('category node', () => {
-        it('state is true when every child selected', () => {
-            // arrange
-            const node = {
-                id: '1',
-                data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                children: [
-                    {   id: '2',
-                        data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                        children: [ getScriptNode('a'), getScriptNode('b') ],
-                    },
-                    {   id: '3',
-                        data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                        children: [ getScriptNode('c') ],
-                    },
-                ],
-            };
-            const selectedScriptNodeIds = [ 'a', 'b', 'c' ];
-            // act
-            const actual = getNewCheckedState(node, selectedScriptNodeIds);
-            // assert
-            expect(actual).to.equal(true);
+    describe('indeterminate', () => {
+        describe('script node', () => {
+            it('false when selected', () => {
+                // arrange
+                const node = getScriptNode();
+                const selectedScriptNodeIds = [ 'a', 'b', node.id, 'c' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.indeterminate).to.equal(false);
+            });
+            it('false when not selected', () => {
+                // arrange
+                const node = getScriptNode();
+                const selectedScriptNodeIds = [ 'a', 'b', 'c' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.indeterminate).to.equal(false);
+            });
         });
-        it('state is false when none of the children is selected', () => {
-            // arrange
-            const node = {
-                id: '1',
-                data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                children: [
-                    {   id: '2',
-                        data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                        children: [ getScriptNode('a'), getScriptNode('b') ],
-                    },
-                    {   id: '3',
-                        data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                        children: [ getScriptNode('c') ],
-                    },
-                ],
-            };
-            const selectedScriptNodeIds = [ 'none', 'of', 'them', 'are', 'selected' ];
-            // act
-            const actual = getNewCheckedState(node, selectedScriptNodeIds);
-            // assert
-            expect(actual).to.equal(false);
-        });
-        it('state is false when some of the children is selected', () => {
-            // arrange
-            const node = {
-                id: '1',
-                data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                children: [
-                    {
-                        id: '2',
-                        data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                        children: [ getScriptNode('a'), getScriptNode('b') ],
-                    },
-                    {
-                        id: '3',
-                        data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
-                        children: [ getScriptNode('c') ],
-                    },
-                ],
-            };
-            const selectedScriptNodeIds = [ 'a', 'c', 'unrelated' ];
-            // act
-            const actual = getNewCheckedState(node, selectedScriptNodeIds);
-            // assert
-            expect(actual).to.equal(false);
+        describe('category node', () => {
+            it('false when all children are selected', () => {
+                // arrange
+                const node = {
+                    id: '1',
+                    data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                    children: [
+                        {   id: '2',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('a'), getScriptNode('b') ],
+                        },
+                        {   id: '3',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('c') ],
+                        },
+                    ],
+                };
+                const selectedScriptNodeIds = [ 'a', 'b', 'c' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.indeterminate).to.equal(false);
+            });
+            it('true when all some are selected', () => {
+                // arrange
+                const node = {
+                    id: '1',
+                    data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                    children: [
+                        {   id: '2',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('a'), getScriptNode('b') ],
+                        },
+                        {   id: '3',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('c') ],
+                        },
+                    ],
+                };
+                const selectedScriptNodeIds = [ 'a' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.indeterminate).to.equal(true);
+            });
+            it('false when no children are selected', () => {
+                // arrange
+                const node = {
+                    id: '1',
+                    data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                    children: [
+                        {   id: '2',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('a'), getScriptNode('b') ],
+                        },
+                        {   id: '3',
+                            data: { type: NodeType.Category, documentationUrls: [], isReversible: false },
+                            children: [ getScriptNode('c') ],
+                        },
+                    ],
+                };
+                const selectedScriptNodeIds = [ 'none', 'of', 'them', 'are', 'selected' ];
+                // act
+                const state = getNewState(node, selectedScriptNodeIds);
+                // assert
+                expect(state.indeterminate).to.equal(false);
+            });
         });
     });
 });
diff --git a/tests/unit/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeTranslator.spec.ts b/tests/unit/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeTranslator.spec.ts
index 06e8ac77..c183ea81 100644
--- a/tests/unit/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeTranslator.spec.ts
+++ b/tests/unit/presentation/Scripts/ScriptsTree/SelectableTree/LiquorTree/NodeWrapper/NodeTranslator.spec.ts
@@ -108,6 +108,7 @@ function getNewNode(): ILiquorTreeNewNode {
     const base = getNode();
     const commonState = {
         checked: false,
+        indeterminate: false,
     };
     return {
         id: base.id,