Improve checkableTreeNode logic

Tests/UnitTests/Models/CheckableTreeNode.cs

@@ -0,0 +1,115 @@
+using COMPASS.Models;
+using COMPASS.Tools;
+using System.Collections.ObjectModel;
+
+namespace Tests.UnitTests.Models
+{
+    [TestClass]
+    public class CheckableTreeNode
+    {
+        private static CheckableTreeNode<Tag>? checkableRoot;
+
+        [ClassInitialize]
+        public static void Initialize(TestContext context)
+        {
+            //Setup
+            Tag root = new()
+            {
+                ID = 1,
+                IsGroup = true,
+                Children = new ObservableCollection<Tag>()
+                {
+                    new() //L1 that is a group
+                    {
+                        ID = 2,
+                        IsGroup = true,
+                        Children = new ObservableCollection<Tag>()
+                        {
+                            new() //some L2 children
+                            {
+                                ID = 3,
+                                IsGroup = false,
+                            },
+                            new() //some L2 children
+                            {
+                                ID = 4,
+                                IsGroup = false,
+                            },
+                        }
+                    },
+                    new() //L1 that is not a group
+                    {
+                        ID = 5,
+                        IsGroup = false,
+                        Children = new ObservableCollection<Tag>()
+                        {
+                            new() //some L2 children
+                            {
+                                ID = 6,
+                                IsGroup = false,
+                            },
+                            new() //some L2 children
+                            {
+                                ID = 7,
+                                IsGroup = false,
+                            },
+                        }
+                    },
+                }
+            };
+
+            checkableRoot = new(root, containerOnly: root.IsGroup);
+            foreach (var item in checkableRoot.Children.Flatten())
+            {
+                item.ContainerOnly = item.Item.IsGroup;
+            }
+        }
+
+        [TestMethod]
+        public void TestDownwardsPropagatiion()
+        {
+            //Downwards false
+            checkableRoot!.IsChecked = false;
+            Assert.IsTrue(checkableRoot!.Children.Flatten().All(child => child.IsChecked == false));
+
+            //Downwards true
+            checkableRoot!.IsChecked = true;
+            Assert.IsTrue(checkableRoot!.Children.Flatten().All(child => child.IsChecked == true));
+        }
+
+        [TestMethod]
+        public void TestUpwardsPropagatiion()
+        {
+            //uncheck all to start
+            checkableRoot!.IsChecked = false;
+
+            var firstChild = checkableRoot.Children[0];
+            var secondChild = checkableRoot.Children[1];
+
+            //check one, all above should become null
+            firstChild.Children.First().IsChecked = true;
+            Assert.IsNull(firstChild.IsChecked);
+            Assert.IsNull(checkableRoot.IsChecked);
+
+            //check the other, should cause parent to be checked
+            firstChild.Children[1].IsChecked = true;
+            Assert.IsTrue(firstChild.IsChecked);
+            Assert.IsNull(checkableRoot.IsChecked);
+
+            //now uncheck children of first, because it is containerOnly, should be unchecked
+            firstChild.Children[0].IsChecked = false;
+            firstChild.Children[1].IsChecked = false;
+            Assert.IsTrue(firstChild.ContainerOnly);
+            Assert.IsFalse(firstChild.IsChecked);
+            Assert.IsFalse(checkableRoot.IsChecked);
+
+            //check second child and uncheck children, because it is NOT containerOnly, should stay checked
+            secondChild.IsChecked = true;
+            secondChild.Children[0].IsChecked = false;
+            secondChild.Children[1].IsChecked = false;
+            Assert.IsFalse(secondChild.ContainerOnly);
+            Assert.IsTrue(secondChild.IsChecked);
+            Assert.IsNull(checkableRoot.IsChecked);
+        }
+    }
+}

src/Models/CheckableTreeNode.cs

@@ -7,10 +7,11 @@ namespace COMPASS.Models
 {
     public class CheckableTreeNode<T> : ObservableObject, IHasChildren<CheckableTreeNode<T>> where T : class, IHasChildren<T>
     {
-        public CheckableTreeNode(T item)
+        public CheckableTreeNode(T item, bool containerOnly)
         {
             _item = item;
-            Children = new(item.Children.Select(child => new CheckableTreeNode<T>(child)));
+            ContainerOnly = containerOnly;
+            Children = new(item.Children.Select(child => new CheckableTreeNode<T>(child, containerOnly)));
 
             Children.CollectionChanged += (_, _) =>
             {
@@ -29,6 +30,12 @@ public T Item
             set => SetProperty(ref _item, value);
         }
 
+        /// <summary>
+        /// Indicates that the node only exists as a container for its children,
+        /// cannot be checked on it's own. Will be unchecked if all children are unchecked.
+        /// </summary>
+        public bool ContainerOnly { get; set; }
+
         private bool? _isChecked = false;
         public bool? IsChecked
         {
@@ -41,17 +48,17 @@ public bool? IsChecked
                     //Propagate changes upward
                     Parent?.Update();
                     //propagate changes downwards
-                    if (value != null)
-                    {
-                        foreach (var child in Children)
-                        {
-                            child.IsChecked = value;
-                        }
-                    }
+                    PropagateDown(value);
                 }
             }
         }
 
+        /// <summary>
+        /// Used to set the checked property without triggering updates up and down
+        /// </summary>
+        /// <param name="value"></param>
+        private void InternalSetChecked(bool? value) => SetProperty(ref _isChecked, value, nameof(IsChecked));
+
         private ObservableCollection<CheckableTreeNode<T>> _children = new();
         public ObservableCollection<CheckableTreeNode<T>> Children
         {
@@ -69,21 +76,41 @@ public ObservableCollection<CheckableTreeNode<T>> Children
 
         public CheckableTreeNode<T>? Parent { get; set; }
 
+        public void PropagateDown(bool? isChecked)
+        {
+            if (isChecked != null)
+            {
+                InternalSetChecked(isChecked);
+                foreach (var child in Children)
+                {
+                    child.PropagateDown(isChecked);
+                }
+            }
+        }
+
         private void Update()
         {
+            bool? newValue = _isChecked;
             if (Children.All(child => child.IsChecked == true))
             {
-                IsChecked = true;
+                newValue = true;
             }
             else if (Children.All(child => child.IsChecked == false))
             {
-                IsChecked = false;
+                if (ContainerOnly)
+                {
+                    newValue = false;
+                }
+                else newValue ??= true; //if it was null (partial check), become full check
             }
             else
             {
-                IsChecked = null;
+                newValue = null;
             }
+
+            InternalSetChecked(newValue);
             Updated?.Invoke(IsChecked);
+            Parent?.Update();
         }
 
         public event Action<bool?>? Updated;

src/ViewModels/Import/ImportFolderViewModel.cs

@@ -120,12 +120,12 @@ private List<string> LetUserFilterToImport(IList<string> toImport)
 
                 //Build the checkable folder Tree
                 IEnumerable<Folder> folderObjects = RecursiveDirectories.Select(f => new Folder(f));
-                CheckableFolders = folderObjects.Select(f => new CheckableTreeNode<Folder>(f)).ToList();
+                CheckableFolders = folderObjects.Select(f => new CheckableTreeNode<Folder>(f, containerOnly: false)).ToList();
 
                 foreach (Folder folder in ExistingFolders)
                 {
                     //first make a checkable Folder with all the subfolders, then uncheck those not in the original
-                    var checkableFolder = new CheckableTreeNode<Folder>(new Folder(folder.FullPath));
+                    var checkableFolder = new CheckableTreeNode<Folder>(new Folder(folder.FullPath), containerOnly: false);
                     var chosenSubFolderPaths = folder.SubFolders.Flatten().Select(sf => sf.FullPath).ToList();
                     foreach (var subFolder in checkableFolder.Children.Flatten())
                     {

src/ViewModels/TagsSelectorViewModel.cs

@@ -57,15 +57,16 @@ public CheckableTreeNode<Tag> TagsRoot
                     //load if not done yet
                     if (!_collection.AllTags.Any()) _collection.LoadTags();
                     //convert to nodes
-                    _tagsRoot = new CheckableTreeNode<Tag>(new Tag())
+                    _tagsRoot = new CheckableTreeNode<Tag>(new Tag(), containerOnly: true)
                     {
                         Children = new(_collection.RootTags
-                            .Select(t => new CheckableTreeNode<Tag>(t)))
+                            .Select(t => new CheckableTreeNode<Tag>(t, containerOnly: t.IsGroup)))
                     };
-                    //init expanded and checked
+                    //init expanded, checked and containr only
                     foreach (var node in _tagsRoot.Children.Flatten())
                     {
                         node.Expanded = node.Item.IsGroup;
+                        node.ContainerOnly = node.Item.IsGroup;
                         node.IsChecked = false;
                     }
                     _tagsRoot.Updated += _ => RaisePropertyChanged(nameof(ImportCount));

src/Views/LeftDockView.xaml

@@ -288,7 +288,7 @@
                                 <StackPanel>
                                     <Button Style="{StaticResource TextButton}" Background="{StaticResource Layer4}" Margin="5"
                                             Command="{Binding ImportTagsFromOtherCollectionsCommand}" HorizontalContentAlignment="Left">
-                                        <TextBlock Text="From Another collection..." TextWrapping="Wrap"/>
+                                        <TextBlock Text="From another collection..." TextWrapping="Wrap"/>
                                     </Button>
                                     <Button Style="{StaticResource TextButton}" Background="{StaticResource Layer4}" Margin="5,0,5,5"
                                             Command="{Binding ImportTagsFromSatchelCommand}" HorizontalContentAlignment="Left">