Fixes #4009 - fix tree ordering

Terminal.Gui/Views/TreeView/Branch.cs

@@ -27,7 +27,7 @@ public Branch (TreeView<T> tree, Branch<T> parentBranchIfAny, T model)
     ///     The children of the current branch.  This is null until the first call to <see cref="FetchChildren"/> to avoid
     ///     enumerating the entire underlying hierarchy.
     /// </summary>
-    public Dictionary<T, Branch<T>> ChildBranches { get; set; }
+    public List<Branch<T>> ChildBranches { get; set; }
 
     /// <summary>The depth of the current branch.  Depth of 0 indicates root level branches.</summary>
     public int Depth { get; }
@@ -275,14 +275,14 @@ public virtual void FetchChildren ()
 
         if (Depth >= tree.MaxDepth)
         {
-            children = Enumerable.Empty<T> ();
+            children = [];
         }
         else
         {
-            children = tree.TreeBuilder.GetChildren (Model) ?? Enumerable.Empty<T> ();
+            children = tree.TreeBuilder.GetChildren (Model) ?? [];
         }
 
-        ChildBranches = children.ToDictionary (k => k, val => new Branch<T> (tree, this, val));
+        ChildBranches = children.Select (o=>new Branch<T> (tree, this, o)).ToList ();
     }
 
     /// <summary>
@@ -340,15 +340,15 @@ public void Refresh (bool startAtTop)
             // we already knew about some children so preserve the state of the old children
 
             // first gather the new Children
-            IEnumerable<T> newChildren = tree.TreeBuilder?.GetChildren (Model) ?? Enumerable.Empty<T> ();
+            T[] newChildren = tree.TreeBuilder?.GetChildren (Model).ToArray () ?? [];
 
             // Children who no longer appear need to go
-            foreach (T toRemove in ChildBranches.Keys.Except (newChildren).ToArray ())
+            foreach (Branch<T> toRemove in ChildBranches.Where (b=>!newChildren.Contains(b.Model)).ToArray ())
             {
                 ChildBranches.Remove (toRemove);
 
                 //also if the user has this node selected (its disappearing) so lets change selection to us (the parent object) to be helpful
-                if (Equals (tree.SelectedObject, toRemove))
+                if (Equals (tree.SelectedObject, toRemove.Model))
                 {
                     tree.SelectedObject = Model;
                 }
@@ -357,17 +357,21 @@ public void Refresh (bool startAtTop)
             // New children need to be added
             foreach (T newChild in newChildren)
             {
+                Branch<T> existingBranch = ChildBranches.FirstOrDefault (b => b.Model.Equals (newChild));
                 // If we don't know about the child, yet we need a new branch
-                if (!ChildBranches.ContainsKey (newChild))
+                if (existingBranch == null)
                 {
-                    ChildBranches.Add (newChild, new Branch<T> (tree, this, newChild));
+                    ChildBranches.Add (new (tree, this, newChild));
                 }
                 else
                 {
                     //we already have this object but update the reference anyway in case Equality match but the references are new
-                    ChildBranches [newChild].Model = newChild;
+                    existingBranch.Model = newChild;
                 }
             }
+
+            // Order the list
+            ChildBranches = ChildBranches.OrderBy (b => newChildren.IndexOf (b.Model)).ToList ();
         }
     }
 
@@ -381,9 +385,9 @@ internal void CollapseAll ()
 
         if (ChildBranches is { })
         {
-            foreach (KeyValuePair<T, Branch<T>> child in ChildBranches)
+            foreach (Branch<T> child in ChildBranches)
             {
-                child.Value.CollapseAll ();
+                child.CollapseAll ();
             }
         }
     }
@@ -395,9 +399,9 @@ internal void ExpandAll ()
 
         if (ChildBranches is { })
         {
-            foreach (KeyValuePair<T, Branch<T>> child in ChildBranches)
+            foreach (Branch<T> child in ChildBranches)
             {
-                child.Value.ExpandAll ();
+                child.ExpandAll ();
             }
         }
     }
@@ -487,9 +491,9 @@ internal void Rebuild ()
             if (IsExpanded)
             {
                 // if we are expanded we need to update the visible children
-                foreach (KeyValuePair<T, Branch<T>> child in ChildBranches)
+                foreach (Branch<T> child in ChildBranches)
                 {
-                    child.Value.Rebuild ();
+                    child.Rebuild ();
                 }
             }
             else
@@ -526,7 +530,7 @@ private bool IsLast ()
             return this == tree.roots.Values.LastOrDefault ();
         }
 
-        return Parent.ChildBranches.Values.LastOrDefault () == this;
+        return Parent.ChildBranches.LastOrDefault () == this;
     }
 
     private static Cell NewCell (Attribute attr, Rune r) { return new Cell { Rune = r, Attribute = new (attr) }; }

Terminal.Gui/Views/TreeView/TreeView.cs

@@ -847,7 +847,7 @@ public IEnumerable<T> GetChildren (T o)
             return new T [0];
         }
 
-        return branch.ChildBranches?.Values?.Select (b => b.Model)?.ToArray () ?? new T [0];
+        return branch.ChildBranches?.Select (b => b.Model)?.ToArray () ?? new T [0];
     }
 
     /// <summary>Returns the maximum width line in the tree including prefix and expansion symbols.</summary>
@@ -1488,7 +1488,7 @@ private IEnumerable<Branch<T>> AddToLineMap (Branch<T> currentBranch, bool paren
 
         if (currentBranch.IsExpanded)
         {
-            foreach (Branch<T> subBranch in currentBranch.ChildBranches.Values)
+            foreach (Branch<T> subBranch in currentBranch.ChildBranches)
             {
                 foreach (Branch<T> sub in AddToLineMap (subBranch, weMatch, out bool childMatch))
                 {

TerminalGuiFluentTesting/GuiTestContext.cs

@@ -22,7 +22,7 @@
     private readonly StringBuilder _logsSb;
     private readonly V2TestDriver _driver;
 
     internal GuiTestContext (Func<Toplevel> topLevelBuilder, int width, int height, V2TestDriver driver)
     {
         IApplication origApp = ApplicationImpl.Instance;
         ILogger? origLogger = Logging.Logger;
@@ -183,7 +183,7 @@
         return WaitIteration ();
     }
 
     public GuiTestContext ScreenShot (string title, TextWriter writer)
     {
         writer.WriteLine (title + ":");
         var text = Application.ToString ();
@@ -243,7 +243,18 @@
     /// <returns></returns>
     public GuiTestContext Then (Action doAction)
     {
-        doAction ();
+        try
+        {
+            doAction ();
+        }
+        catch(Exception)
+        {
+            Stop ();
+            _hardStop.Cancel();
+
+            throw;
+
+        }
 
         return this;
     }
@@ -320,7 +331,7 @@
         return this;
     }
 
     public GuiTestContext Down ()
     {
         switch (_driver)
         {
@@ -360,6 +371,7 @@
                 {
                     SendNetKey (k);
                 }
+                WaitIteration ();
                 break;
             default:
                 throw new ArgumentOutOfRangeException ();
@@ -550,4 +562,25 @@
 
         WaitIteration ();
     }
+
+    /// <summary>
+    /// Sets the input focus to the given <see cref="View"/>.
+    /// Throws <see cref="ArgumentException"/> if focus did not change due to system
+    /// constraints e.g. <paramref name="toFocus"/>
+    /// <see cref="View.CanFocus"/> is <see langword="false"/>
+    /// </summary>
+    /// <param name="toFocus"></param>
+    /// <returns></returns>
+    /// <exception cref="ArgumentException"></exception>
+    public GuiTestContext Focus (View toFocus)
+    {
+        toFocus.FocusDeepest (NavigationDirection.Forward, TabBehavior.TabStop);
+
+        if (!toFocus.HasFocus)
+        {
+            throw new ArgumentException ("Failed to set focus, FocusDeepest did not result in HasFocus becoming true. Ensure view is added and focusable");
+        }
+
+        return WaitIteration ();
+    }
 }

Tests/IntegrationTests/FluentTests/BasicFluentAssertionTests.cs

@@ -1,5 +1,4 @@
-using System.Text;
-using Terminal.Gui;
+using Terminal.Gui;
 using TerminalGuiFluentTesting;
 using Xunit.Abstractions;
 
@@ -9,17 +8,6 @@ public class BasicFluentAssertionTests
 {
     private readonly TextWriter _out;
 
-    public class TestOutputWriter : TextWriter
-    {
-        private readonly ITestOutputHelper _output;
-
-        public TestOutputWriter (ITestOutputHelper output) { _output = output; }
-
-        public override void WriteLine (string? value) { _output.WriteLine (value ?? string.Empty); }
-
-        public override Encoding Encoding => Encoding.UTF8;
-    }
-
     public BasicFluentAssertionTests (ITestOutputHelper outputHelper) { _out = new TestOutputWriter (outputHelper); }
 
     [Theory]

Tests/IntegrationTests/FluentTests/TestOutputWriter.cs

@@ -0,0 +1,15 @@
+using System.Text;
+using Xunit.Abstractions;
+
+namespace IntegrationTests.FluentTests;
+
+public class TestOutputWriter : TextWriter
+{
+    private readonly ITestOutputHelper _output;
+
+    public TestOutputWriter (ITestOutputHelper output) { _output = output; }
+
+    public override void WriteLine (string? value) { _output.WriteLine (value ?? string.Empty); }
+
+    public override Encoding Encoding => Encoding.UTF8;
+}

Tests/IntegrationTests/FluentTests/TreeViewFluentTests.cs

@@ -0,0 +1,162 @@
+using Terminal.Gui;
+using TerminalGuiFluentTesting;
+using Xunit.Abstractions;
+
+namespace IntegrationTests.FluentTests;
+
+public class TreeViewFluentTests
+{
+    private readonly TextWriter _out;
+
+    public TreeViewFluentTests (ITestOutputHelper outputHelper) { _out = new TestOutputWriter (outputHelper); }
+
+    [Theory]
+    [ClassData (typeof (V2TestDrivers))]
+    public void TreeView_AllowReOrdering (V2TestDriver d)
+    {
+        var tv = new TreeView
+        {
+            Width = Dim.Fill (),
+            Height = Dim.Fill ()
+        };
+
+        TreeNode car;
+        TreeNode lorry;
+        TreeNode bike;
+
+        var root = new TreeNode ("Root")
+        {
+            Children =
+            [
+                car = new ("Car"),
+                lorry = new ("Lorry"),
+                bike = new ("Bike")
+            ]
+        };
+
+        tv.AddObject (root);
+
+        using GuiTestContext context =
+            With.A<Window> (40, 10, d)
+                .Add (tv)
+                .Focus (tv)
+                .WaitIteration ()
+                .ScreenShot ("Before expanding", _out)
+                .Then (() => Assert.Equal (root, tv.GetObjectOnRow (0)))
+                .Then (() => Assert.Null (tv.GetObjectOnRow (1)))
+                .Right ()
+                .ScreenShot ("After expanding", _out)
+                .Then (() => Assert.Equal (root, tv.GetObjectOnRow (0)))
+                .Then (() => Assert.Equal (car, tv.GetObjectOnRow (1)))
+                .Then (() => Assert.Equal (lorry, tv.GetObjectOnRow (2)))
+                .Then (() => Assert.Equal (bike, tv.GetObjectOnRow (3)))
+                .Then (
+                       () =>
+                       {
+                           // Re order
+                           root.Children = [bike, car, lorry];
+                           tv.RefreshObject (root);
+                       })
+                .WaitIteration ()
+                .ScreenShot ("After re-order", _out)
+                .Then (() => Assert.Equal (root, tv.GetObjectOnRow (0)))
+                .Then (() => Assert.Equal (bike, tv.GetObjectOnRow (1)))
+                .Then (() => Assert.Equal (car, tv.GetObjectOnRow (2)))
+                .Then (() => Assert.Equal (lorry, tv.GetObjectOnRow (3)))
+                .WriteOutLogs (_out);
+
+        context.Stop ();
+    }
+
+    [Theory]
+    [ClassData (typeof (V2TestDrivers))]
+    public void TreeViewReOrder_PreservesExpansion (V2TestDriver d)
+    {
+        var tv = new TreeView
+        {
+            Width = Dim.Fill (),
+            Height = Dim.Fill ()
+        };
+
+        TreeNode car;
+        TreeNode lorry;
+        TreeNode bike;
+
+        TreeNode mrA;
+        TreeNode mrB;
+
+        TreeNode mrC;
+
+        TreeNode mrD;
+        TreeNode mrE;
+
+        var root = new TreeNode ("Root")
+        {
+            Children =
+            [
+                car = new ("Car")
+                {
+                    Children =
+                    [
+                        mrA = new ("Mr A"),
+                        mrB = new ("Mr B")
+                    ]
+                },
+                lorry = new ("Lorry")
+                {
+                    Children =
+                    [
+                        mrC = new ("Mr C")
+                    ]
+                },
+                bike = new ("Bike")
+                {
+                    Children =
+                    [
+                        mrD = new ("Mr D"),
+                        mrE = new ("Mr E")
+                    ]
+                }
+            ]
+        };
+
+        tv.AddObject (root);
+        tv.ExpandAll ();
+
+        using GuiTestContext context =
+            With.A<Window> (40, 13, d)
+                .Add (tv)
+                .WaitIteration ()
+                .ScreenShot ("Initial State", _out)
+                .Then (() => Assert.Equal (root, tv.GetObjectOnRow (0)))
+                .Then (() => Assert.Equal (car, tv.GetObjectOnRow (1)))
+                .Then (() => Assert.Equal (mrA, tv.GetObjectOnRow (2)))
+                .Then (() => Assert.Equal (mrB, tv.GetObjectOnRow (3)))
+                .Then (() => Assert.Equal (lorry, tv.GetObjectOnRow (4)))
+                .Then (() => Assert.Equal (mrC, tv.GetObjectOnRow (5)))
+                .Then (() => Assert.Equal (bike, tv.GetObjectOnRow (6)))
+                .Then (() => Assert.Equal (mrD, tv.GetObjectOnRow (7)))
+                .Then (() => Assert.Equal (mrE, tv.GetObjectOnRow (8)))
+                .Then (
+                       () =>
+                       {
+                           // Re order
+                           root.Children = [bike, car, lorry];
+                           tv.RefreshObject (root);
+                       })
+                .WaitIteration ()
+                .ScreenShot ("After re-order", _out)
+                .Then (() => Assert.Equal (root, tv.GetObjectOnRow (0)))
+                .Then (() => Assert.Equal (bike, tv.GetObjectOnRow (1)))
+                .Then (() => Assert.Equal (mrD, tv.GetObjectOnRow (2)))
+                .Then (() => Assert.Equal (mrE, tv.GetObjectOnRow (3)))
+                .Then (() => Assert.Equal (car, tv.GetObjectOnRow (4)))
+                .Then (() => Assert.Equal (mrA, tv.GetObjectOnRow (5)))
+                .Then (() => Assert.Equal (mrB, tv.GetObjectOnRow (6)))
+                .Then (() => Assert.Equal (lorry, tv.GetObjectOnRow (7)))
+                .Then (() => Assert.Equal (mrC, tv.GetObjectOnRow (8)))
+                .WriteOutLogs (_out);
+
+        context.Stop ();
+    }
+}